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

Stop executeDeletions when there is nothing to to delete anymore #962

Merged
merged 1 commit into from
Mar 23, 2014
Merged

Stop executeDeletions when there is nothing to to delete anymore #962

merged 1 commit into from
Mar 23, 2014

Conversation

zluiten
Copy link
Contributor

@zluiten zluiten commented Feb 25, 2014

A small improvement in UOW::commit. Only continue executing the for-loop when there's something to delete. This may save quite some calls to UOW::executeDeletions.

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-2999

We use Jira to track the state of pull requests and the versions they got
included in.

@@ -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) {
Copy link
Member

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 ?

Copy link
Contributor Author

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.

Copy link
Member

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

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'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.

@Ocramius
Copy link
Member

@Netiul being picky: how many calls do you save here?

@zluiten
Copy link
Contributor Author

zluiten commented Feb 27, 2014

Hm, seems my comment is gone. I thought I already responded to you.
Anyway, @Ocramius, in my case it was between 5 and 23 when removing different kind of entities. Not huge, but still, those extra calls seem useless.

beberlei added a commit that referenced this pull request Mar 23, 2014
Stop executeDeletions when there is nothing to to delete anymore
@beberlei beberlei merged commit a03c8da into doctrine:master Mar 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants