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

Refactor each() to foreach() for compatibility with PHP 7.2 #1377

Merged
merged 1 commit into from Feb 2, 2017

Conversation

drbyte
Copy link
Member

@drbyte drbyte commented Jan 31, 2017

An RFC for PHP 7.2 has been voted on for inclusion in PHP 7.2. It proposes to deprecate the each() function for PHP 7.2 and removes it in 8.0.

The each() function is exponentially slower and wastes resources vs foreach(), so we're refactoring it now whether PHP 7.2 finally does adopt it or not.

Things to note when refactoring:

  1. foreach() doesn't need a reset() to be called before it runs, so those can be removed.
  2. There are 3 syntax formats, depending on how the parameters are presented in the list() call:

a) while(list($key, $value) = each($foo))
This becomes foreach($foo as $key => $value)

b) while(list(, $value) = each($foo))
This becomes foreach($foo as $value)

c) while(list($key, ) = each($foo))
This becomes foreach($foo as $key => $value)

An RFC for PHP 7.2 has been voted on for inclusion in PHP 7.2.  It proposes to deprecate the `each()` function for PHP 7.2 and removes it in 8.0.

The `each()` function is exponentially slower and wastes resources vs `foreach()`, so we're refactoring it now whether PHP 7.2 finally does adopt it or not.
@zcwilt
Copy link
Member

zcwilt commented Feb 1, 2017

👍

1 similar comment
@ajeh
Copy link
Member

ajeh commented Feb 1, 2017

👍

@drbyte
Copy link
Member Author

drbyte commented Feb 1, 2017

Note: our various imported vendor packages we leverage will require updates; will cross that bridge when they push new versions.

@zcwilt zcwilt merged commit faa1d8d into zencart:v160 Feb 2, 2017
@drbyte drbyte deleted the refactor-each()-for-php72 branch February 2, 2017 17:42
@dakanji
Copy link

dakanji commented May 5, 2017

@drbyte, How would you suggest tackling instances where there is no while loop involved?

Examples:
a) list($key, $value) = each($foo)

b) list(, $value) = each($foo)

c) list($key, ) = each($foo)

@drbyte
Copy link
Member Author

drbyte commented May 5, 2017

I'm not aware of any such use cases in actual Zen Cart code, old or new.

Reading http://php.net/manual/en/function.each.php and http://php.net/manual/en/function.list.php offer some insight, particularly the fact that each only handles numeric indices, not named indices. I suspect you'll end up just referring to $foo[0] and $foo[1] etc, or some variation of that.

@dakanji
Copy link

dakanji commented May 6, 2017

Thanks. Came across these elsewhere and was looking for correct solutions when I found your post here.

For the record,

a) list($key, $value) = each($foo) is equivalent to:
$key = array_keys($foo)[0];
$value = array_values($foo)[0];


b) list(, $value) = each($foo) is equivalent to:
$value = array_values($foo)[0];

c) list($key, ) = each($foo) is equivalent to:
$key = array_keys($foo)[0];

... and list($key) = each($foo) is equivalent to Example C

@drbyte
Copy link
Member Author

drbyte commented May 6, 2017

Agreed. Thanks for posting these.

It's unfortunate there's no shorter and equally expressive version as each() when used outside a loop.

@torvista
Copy link
Member

I'm using php7.2 for development, PHPstorm marks "each" as deprecated.
My production site/hosting is already on 7.1.7.
Given the widespread use of "each" in 1.55 (>200 instances), and 7.2 due in November, what is the plan?

@drbyte
Copy link
Member Author

drbyte commented Jul 28, 2017

We were discussing that earlier this week. We'll be tagging a new release.

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.

None yet

5 participants