move the getEntityManager, only get it if needed #5828
Conversation
| @@ -135,10 +135,10 @@ a ``postPersist`` method, which will be called when the event is dispatched:: | |||
| public function postPersist(LifecycleEventArgs $args) | |||
| { | |||
| $entity = $args->getEntity(); | |||
| $entityManager = $args->getEntityManager(); | |||
|
|
|||
| // perhaps you only want to act on some "Product" entity | |||
| if ($entity instanceof Product) { | |||
There was a problem hiding this comment.
i would prefer returning early.
if (!$entity instanceof Product) { return; }
There was a problem hiding this comment.
this is not so clear IMO, maybe you want to check for another Type later?
There was a problem hiding this comment.
returning early makes the code much more readable.
http://programmers.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement
it's not part of the symfony Styleguide but this PR is about readability.
if you want to check for multiple types you probably violate the SRP, so just don't to it :)
There was a problem hiding this comment.
While the guard style is nice, I think the current syntax is more clear for people not familair with this style of guarding (and I assume lots of beginners are not familair with this syntax).
There was a problem hiding this comment.
I generally would be pro "returning early" - i.e. reversing the logic here. I see too many nested if statements in people's code - teaching them to return is a tiny win.
There was a problem hiding this comment.
I am in favour of return early here too.
|
👍 I think we should add a new line before the |
|
Looks fine to me 👍 |
|
Thanks Oskar. |
…ark) This PR was submitted for the 2.7 branch but it was merged into the 2.3 branch instead (closes #5828). Discussion ---------- move the getEntityManager, only get it if needed if this will be merged, please be sure, that this is the correct branch! Thank you Commits ------- 1498140 move the getEntityManager, only get it if needed
* origin/2.7: [#5828] Return early move the getEntityManager, only get it if needed Update front controller Tell about SYMFONY__TEMPLATING__HELPER__CODE__FILE_LINK_FORMAT Added new security advisories to the docs Fixed some wrong line numbers in doctrine.rst [Book][Templating] Update absolute URL asset to match 2.7 Update debug_formatter.rst Removed the use of ContainerAware class Book: Update Service Container Documentation Removed an outdate paragraph minor lang tweak Fixes done automatically by the docbot updated sentence Removed "http_basic" config from the login form cookbook
if this will be merged, please be sure, that this is the correct branch!
Thank you