Skip to content
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

Add QueryBuilder vs DQL section #5637

Closed
wants to merge 1 commit into from

Conversation

bocharsky-bw
Copy link
Contributor

Describe difference between QueryBuilder and DQL usage.

Q A
Doc fix? no
New docs? yes
Applies to all
Fixed tickets none

@@ -793,6 +793,16 @@ For more information on Doctrine's Query Builder, consult Doctrine's

.. _book-doctrine-custom-repository-classes:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The label must be moved (it belongs to the following section).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved it below

@xabbuh
Copy link
Member

xabbuh commented Aug 19, 2015

👍

@bocharsky-bw
Copy link
Contributor Author

Thanks for help with text tweaking!

@wouterj
Copy link
Member

wouterj commented Aug 19, 2015

Nice! It's always good to explain when to use which option.

However, the section isn't really a versus anymore, it just explains the use-cases for the QueryBuilder. What about merging it with the first paragraph of the QB section?

-Instead of writing a DQL string, you can alternatively use a helpful object called
-the ``QueryBuilder`` to build that string for you::
+Instead of writing a DQL string, you can use a helpful object called the
+``QueryBuilder`` to build that string for you. This is usefull when the actual query
+depends on dynamic conditions, as your code soon becomes hard to read with
+DQL as you start to concatenate strings.
+
+.. code-block:: php

(I also removed "atlernatively" in the first sentence, as it was redundant because of "Instead of" at the start of the sentence)

Ideally, we would show an example that shows dynamic query building, but that hasn't to be changed in this PR.

@xabbuh
Copy link
Member

xabbuh commented Aug 19, 2015

Ah indeed, I missed the fact that we have that paragraph. The suggested change really fits better.

@bocharsky-bw
Copy link
Contributor Author

@wouterj I applied your comment

Instead of writing a DQL string, you can alternatively use a helpful object called
the ``QueryBuilder`` to build that string for you::
Instead of writing a DQL string, you can use a helpful object called the
``QueryBuilder`` to build that string for you. This is usefull when the actual query
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useful

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@xabbuh
Copy link
Member

xabbuh commented Aug 21, 2015

👍

1 similar comment
@wouterj
Copy link
Member

wouterj commented Aug 21, 2015

👍

@xabbuh
Copy link
Member

xabbuh commented Aug 22, 2015

Thank you Victor.

xabbuh added a commit that referenced this pull request Aug 22, 2015
This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #5637).

Discussion
----------

Add QueryBuilder vs DQL section

Describe difference between QueryBuilder and DQL usage.

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | yes
| Applies to    | all
| Fixed tickets | none

Commits
-------

cfc5873 Add explanation when to use QueryBuilder instead of DQL
@xabbuh
Copy link
Member

xabbuh commented Aug 22, 2015

@bocharsky-bw FYI, I merged your pull request into the 2.3 branch. That's why you see it as closed.

@xabbuh xabbuh closed this Aug 22, 2015
@bocharsky-bw
Copy link
Contributor Author

@xabbuh Thanks for information. Will it merge to other branches too?

@wouterj
Copy link
Member

wouterj commented Aug 22, 2015

Yes, we merge 2.3 into the newer branches from time to time

@bocharsky-bw
Copy link
Contributor Author

Ok, then I think I could delete this branch patch

@bocharsky-bw bocharsky-bw deleted the patch-4 branch August 22, 2015 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants