Skip to content
This repository has been archived by the owner on Jul 28, 2022. It is now read-only.

using each() is deprecated since PHP 7.2 #307

Merged
merged 4 commits into from
Feb 12, 2018

Conversation

OskarStark
Copy link
Member

I am targeting this branch, because its BC.

Closes #306

Changelog

### Fixed
- `each()` is deprecated since PHP 7.2

@OskarStark OskarStark changed the title using each() is depracted since PHP 7.2 using each() is deprected since PHP 7.2 Jan 23, 2018
@OskarStark OskarStark changed the title using each() is deprected since PHP 7.2 using each() is deprecated since PHP 7.2 Jan 23, 2018
@@ -79,10 +79,8 @@ public function onEvent(Event $event = null)
{
reset($this->messages);
Copy link
Member

Choose a reason for hiding this comment

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

This line is probably unnecesary too

$this->handle($message, $this->dispatcher);

unset($this->messages[$key]);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should clear this variable after the foreach? To avoid changing behavior

@OskarStark OskarStark force-pushed the fixed-deprecation branch 5 times, most recently from 9bf1822 to 3d9c3e3 Compare January 23, 2018 09:09
@OskarStark
Copy link
Member Author

@jordisala1991 looks like the each and the foreach does not result in the same behavior :(

@jordisala1991
Copy link
Member

why not?

@@ -79,7 +79,7 @@ public function onEvent(Event $event = null)
{
reset($this->messages);

while (list($key, $message) = each($this->messages)) {
foreach ($this->messages as $key => $message) {
$this->handle($message, $this->dispatcher);

unset($this->messages[$key]);
Copy link
Member

Choose a reason for hiding this comment

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

You cant unset like this in a foreach, you should move this line outside the foreach in the form:

$this->messages = [];

Copy link
Member Author

Choose a reason for hiding this comment

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

You cant unset like this in a foreach

why not? oO

Copy link
Member

@jordisala1991 jordisala1991 Jan 23, 2018

Choose a reason for hiding this comment

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

Because you are removing elements on the foreach and php no longer knows what element is next.

On the old code it was doing a each until there are no elements on the array. That's why it was working before

Copy link
Member

Choose a reason for hiding this comment

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

Other option would be doing a while with "there are elements on the array" condition, but IMO a foreach is cleaner

Copy link
Member Author

Choose a reason for hiding this comment

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

still failing :(

@jordisala1991
Copy link
Member

Can you try something like this:

while(!empty($this->messages)) {
    $message = array_shift($this->messages);

    $this->handle(...);
}

and you dont need to clear the array anymore

@OskarStark
Copy link
Member Author

Could you provide a PR against mine?

@jordisala1991
Copy link
Member

@OskarStark OskarStark#1 there you go

@OskarStark
Copy link
Member Author

Thank you, currently need a lot of time for my son 😉

@kunicmarko20
Copy link
Contributor

could you fix flintCI if you have time

@OskarStark
Copy link
Member Author

did it via the editor on github.com ;-)

@jordisala1991
Copy link
Member

Please review @sonata-project/contributors

@greg0ire greg0ire merged commit 66771ea into sonata-project:3.x Feb 12, 2018
@greg0ire
Copy link
Contributor

Thanks @OskarStark !

@OskarStark OskarStark deleted the fixed-deprecation branch February 12, 2018 11:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

php 7.2 The each() function is deprecated
4 participants