mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
findOneBy: interface and implementation do not match #6443
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @LucHamers on GitHub (Apr 12, 2020).
Support Question
The method findOneBy ist implemented in Doctrine\ORM\EntityRepository as follows:
It implements the interface Doctrine\Persistence\ObjectRepository, in which the method is defined as follows:
In the interface the parameter $orderBy is missing. Is this a bug?
@SenseException commented on GitHub (Apr 12, 2020):
Persistence is a different project than ORM and others that are depending on Persistence don't need a second argument. I agree that this could be solved for a new major release if the interface would be in ORM.
PHP allows to have additional arguments compared to an interface if those have a default value.
@LucHamers commented on GitHub (Apr 12, 2020):
Technically this is no issue, using the parameter works fine. Problem is, that the phpstorm shows a warning that there is a parameter where there shouldn't be one. And in my opinion the parameter is important, and as user I cannot find it when my IDE only shows the interface without this parameter.
@greg0ire commented on GitHub (Apr 12, 2020):
You have no guarantee about that though, correct? If you do, then you have to let PHPStorm figure out that the object you are using is an
EntityRepositorysomehow, be it with checks, assertions,@varannotations or type declarations.@LucHamers commented on GitHub (Apr 13, 2020):
In my opinion, the interface should reflect the implementation and vice versa. If the EntityRepository needs a different interface (and it obviously does), then it also should have an interface WITH the $orderBy parameter in the findOneBy method. That this problem shows in the IDE, is just a symptom. There probably is some kind of workaround for the IDE via annotations or whatever, but I think that the root cause of this should be addressed.
@greg0ire commented on GitHub (Apr 13, 2020):
Me too, but before that we should try to understand why things were done like this.
Here is the commit that introduces the interface:
59e6b8c6edHere is the PR that introduces the implementation: https://github.com/doctrine/orm/pull/504. Sadly, the commit message does not help. Let it be a reminder that commit messages consisting only of links to Github, or worse, JIRA issues should be forbidden.
EDIT: found the correct URL: https://github.com/doctrine/orm/issues/1850
Apparently, adding this argument to the interface was considered
Also see: https://github.com/doctrine/common/pull/741
Are there other implementations that also have this parameter by the way? And can you show the code sample that causes your issue?
@LucHamers commented on GitHub (Apr 13, 2020):
I don't know if there are other implementations of this interface. I am just a doctrine user. In my code, I use it to find the highest number in a table:
@greg0ire commented on GitHub (Apr 13, 2020):
The good news is, you don't have to be a Doctrine maintainer to answer that question, being a Github user will be enough: https://github.com/search?q=org%3Adoctrine+%22implements+ObjectRepository%22&type=Code
When I read your code, I feel that the issue would go away by changing this piece of code:
8c259ea5cb/lib/Doctrine/ORM/EntityManager.php (L693)Let's either replace
|with&, or just dropObjectManager|entirely, since it provides no additional information.But then again, this line of code has a history: https://github.com/doctrine/orm/pull/6780
@greg0ire commented on GitHub (Apr 13, 2020):
Maybe we should somehow discourage the use of
getRepository, see this post of how you could change your code to completely avoid that kind of issues, and get some other advantages in the process: https://www.tomasvotruba.com/blog/2017/10/16/how-to-use-repository-with-doctrine-as-service-in-symfony/EDIT:
yes we should. If you want to help with this, consider contributing documentation updates showing other, better ways of doing things.