DDC-1536: GH-213: QueryBuilder::getQuery #2169

Closed
doctrinebot opened this Issue Dec 13, 2011 · 8 comments

4 participants

@doctrinebot

Jira issue originally created by user @beberlei:

Pull-Request was automatically synchronized: #213

Hi there,
I'm wondering myself if the QueryBuilder::getQuery is a good method name.
Actually, a getMethodObject makes me think that I can use many times the getMethodObject and work with the same object.
e.g :

$qb->getQuery()->useResultCache (true);
$qb->getQuery()->execute();

In my code, the problem is that getQuery return a new instance of Query each time that I call getQuery.
Furthermore, the getQuery method invoke a createQuery method.

So, maybe the QueryBuiler::getQuery can store a reference to the created Query instance, and if the QueryBuilder::getQuery is call many times without any change return the stored instance.
If the QueryBuilder::getQuery have to generate a new Query, throw an exception or something like that.

Because of a change to QueryBuilder::createQuery is a major change, I don't think that It could be the solution, but it's a reflexion around this.

What do you think about that ?

@doctrinebot

Comment created by @guilhermeblanco:

This includes a BC break.
Marking as won't fix.

@doctrinebot

Issue was closed with resolution "Won't Fix"

@doctrinebot doctrinebot added this to the 2.2 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
@doctrinebot doctrinebot added the Bug label Dec 7, 2015
@peterjmit

I would love to see this fixed in Doctrine 3. I am a long time user of doctrine (4+ years now) and a combination of the get prefix and the fluent interface of QueryBuilder led me to believe that getQuery would always return the same instance of Doctrine\ORM\Query.

A bad assumption to make, but one that resulted in a 2yr+ bug for us and a bit of a sharp edge!

@guilhermeblanco is this something you would consider revisiting for v3?

@guilhermeblanco
Doctrine member

There're a number of changes we're planning to do for v3. And yes, this is one of them. =)

@peterjmit

@guilhermeblanco brilliant, thanks 😄

@Ocramius
Doctrine member

This looks invalid to me btw: no interface can guarantee returning the same object over multiple calls (there is no way to make that type safe anyway).

The QueryBuilder already defies a lot of rules of good OO, but it's the nature of this object. getQuery is actually a cast operation, and doesn't cache the returned value (correctly, IMO).

@peterjmit

@Ocramius I understand your point and I don't necessarily think that getQuery should cache the result. I think this is less to do with OO/type safety, and more to do with DX/consistency.

Doctrine already has methods prefixed with create that return newly constructed objects, I think that signifies that the difference between get and create would be significant**, especially in this example where QueryBuilder::getQuery is a wrapper for EntityManagerInterface::createQuery.

** I mean specifically for methods that return objects

@Ocramius
Doctrine member

@peterjmit agree, we should probably move away from get* methods, and instead call it toQuery (or similar), which indeed expresses a completely different intent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment