Skip to content

[7.x] Optimize Arr::forget() method#32294

Closed
BSN4 wants to merge 2 commits into
laravel:7.xfrom
BSN4:7.x
Closed

[7.x] Optimize Arr::forget() method#32294
BSN4 wants to merge 2 commits into
laravel:7.xfrom
BSN4:7.x

Conversation

@BSN4
Copy link
Copy Markdown
Contributor

@BSN4 BSN4 commented Apr 8, 2020

This PR replaces while:array_shift with foreach:unset just like the other two PRs did #32192 & #32282

@nguyenyou
Copy link
Copy Markdown
Contributor

https://www.php.net/manual/en/function.current.php#116128
This note on PHP documentation said:

If you do current() after using unset() on foreach statement, you can get FALSE in PHP version 5.2.4 and above.

Don't know if this should be considered...

@BSN4
Copy link
Copy Markdown
Contributor Author

BSN4 commented Apr 8, 2020

didn't saw this not I guess we need array_shift ?

@BSN4
Copy link
Copy Markdown
Contributor Author

BSN4 commented Apr 8, 2020

@nguyenyou tested current and reset both works on php 7.4
reset is guaranteed to be the first item while current is based on the pointer at calltime

@taylorotwell
Copy link
Copy Markdown
Member

Let's hold off on this for now. I don't want to introduce subtle bugs for very little real world performance gain.

@BSN4
Copy link
Copy Markdown
Contributor Author

BSN4 commented Apr 8, 2020

@taylorotwell thanks

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.

3 participants