-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace query builder with DQL #143
Conversation
Even if this method call is not strictly necessary, don't you think it improves code readability? Removing it looks like "magic" and Symfony always prefer "explicit". What do you think? |
Yes, it looks a bit magically. However with I based this PR on Querying for Objects Using Doctrine's Query Builder but it's a tutorial, not best practices. Also I surfed knpuniversity, Ryan don't use As for me, the Do you think we should keep |
👍 I think we should remove |
@javiereguiluz Why is the query builder used at all in this example? Imho we can simply use a plain DQL query here as the query is not built based on some argument. |
@xabbuh yes, I think DQL should be used in this example. By the way, anyone knows a good reference in the official Doctrine documentation that explains when to use each of their different ways to build queries? I'd like to add a help note linking to that article. Thanks. |
@javiereguiluz I think Symfony docs has nice explanation about quering with DQL vs Query Builder. |
Honestly, I find the explanation very lacking. For example, when they explain the "query builder":
But they never say why the query builder is better or worse and when to use it or avoid it. |
It'll be nice to improve Symfony docs for this. |
The minor problem is that in PhpStorm I have autocompletion by property names with Query Builder what I haven't with DQL :( |
FROM AppBundle:Post p | ||
WHERE p.publishedAt <= :now | ||
ORDER BY p.publishedAt DESC | ||
") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use single quotes instead of double quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I'm interesting why does it better in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the Symfony code style which we should imho follow here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this explanation, I replaced quotes
👍 |
@javiereguiluz What do you think about add next explanation?
|
@bocharsky-bw I think it's a very good starting point to make a pull request to the symfony-docs repo. My only concern is that phrases are too long :) But please, make the pull request and our cool doc managers will guide you to tweak the original text. Thanks! |
I created PR symfony/symfony-docs#5637 |
Thank you @bocharsky-bw. |
This PR was merged into the master branch. Discussion ---------- Replace query builder with DQL The `select` and `from` methods are optional Commits ------- 83e5b9a Replace query builder with DQL
DQL has better performance over Query Builder? |
@Rasanga the query builder will ultimately produce the DQL string. So if the query is not dynamic, you only perform extra work for no additional value if you build the string dynamically with many method calls. |
The
select
andfrom
methods are optional