-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Stop executeDeletions when there is nothing to to delete anymore #962
Conversation
Hello, thank you for creating this pull request. I have automatically opened an issue http://www.doctrine-project.org/jira/browse/DDC-2999 We use Jira to track the state of pull requests and the versions they got |
@@ -373,7 +373,7 @@ public function commit($entity = null) | |||
|
|||
// Entity deletions come last and need to be in reverse commit order | |||
if ($this->entityDeletions) { | |||
for ($count = count($commitOrder), $i = $count - 1; $i >= 0; --$i) { | |||
for ($count = count($commitOrder), $i = $count - 1; $i >= 0, $this->entityDeletions; --$i) { |
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.
shouldn't this be && !empty($this->entityDeletions)
instead ?
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.
That's what I did at first, but then I equalized it with the if-statement right before this line.
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.
No, you don't. You use a comma (separating statements), while I'm using a boolean operator to have a single condition
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.
I'm sorry, I didn't notice the boolean operator, so I thought you were referring to !empty($this->entityDeletions)
vs just $this->entityDeletions
. That's what my comment was about.
But you're right, with comma separation, only the result of the last statement is used for evaluation.
@Netiul being picky: how many calls do you save here? |
Hm, seems my comment is gone. I thought I already responded to you. |
Stop executeDeletions when there is nothing to to delete anymore
A small improvement in
UOW::commit
. Only continue executing the for-loop when there's something to delete. This may save quite some calls toUOW::executeDeletions
.