Modifying an array during a foreach loop [closed]

Question

This question is always on my mind. I usually alter the array while looping through its keys, based on my gut feelings:

foreach (array_keys($array) as $key) { $array[$key] = perform_changes_on($array[$key]); } 

I'm aware of other methods, though. The array item can be passed by reference on the foreach call:

foreach ($array as &$item) { $item = perform_changes_on($item); } 

Finally, the array can be modified directly during the loop:

foreach ($array as $key => $value) { $array[$key] = perform_changes_on($value); } 

What are the performance and security implications of each approach? Is there a recommended approach?

UPDATE: What I'm actually worried about is that the two last approaches modify the array while I'm looping it.

Answer 1

If you are worried about modifying the array while looping, don't because foreach copies the array. Try

$array = array('foo'); foreach ($array as $k => &$item) { $array[] = 'bar'; } var_dump($array); 

And see it terminates just fine. foreach ($array as $k => &$v) is a shorthand for foreach (array_keys($array) as $k) $v = &$array[$k] so while there still is a copy of the array (that's why I used &$item in my example so you can see, if you modify the array then it'll be modified in the reference!

$array = array('foo', 'bar'); foreach ($array as $k => $item) { echo "$item\n"; if (!$k) { $array[1] = 'baz'; } } $array = array('foo', 'bar'); foreach ($array as $k => &$item) { echo "$item\n"; if (!$k) { $array[1] = 'baz'; } } 

the first dump foo and bar, the second foo and baz.

Answer 2

This sounds like a place for array_map().

$array = array_map('perform_changes_on', $array); 

Answer 3

When using foreach($array as &$item) never ever forget the unset($item); after the foreach or you will get into serious trouble trying to use $item later. It should be habitual to avoid this trap.

In general you should avoid foreach ...& and do array_walk($array, function (&$item) {... so that the reference is strictly confined inside the closure.

Answer 4

I can't really help you with the performance bit, only tell you to wrap them in microtime() tags and see which one performs better for you. Systems are slightly different. The one suggestion I can give you is to remove array_keys() from your code.

!!UPDATE!!

If you were following Corbin and my argument below, then I finally have an answer for you. I was getting for and foreach confused. For and while loops do call any functions passed in as arguments on every iteration, foreach does not. Which is why its better to call functions such as strlen() and count(), just to give a couple of examples, outside of a for or while loop. The overhead we were experiencing was not from foreach but from array_keys(). When array_keys() is called it must generate a new array, which is why it is almost twice as slow. So it is best to drop the array_keys() function all-together and just iterate over the array and retrieve the key value pair. Sorry for any confusion this may have caused.

Sources:

• http://www.phpbench.com/
• https://stackoverflow.com/questions/3430194/performance-of-for-vs-foreach-in-php
• https://stackoverflow.com/questions/1740575/php5-does-the-copy-of-an-array-for-a-foreach-incur-overhead

!!END OF UPDATE!!

To the best of my knowledge there is no security risk with any of those implementations. You are iterating a construct that already exists. Any security issues would have happened before this point. Except of course if you are using user supplied data, such as GET and POST. These you should sanitize and validate before using, which is something you could do with one of those foreach loops. Or you could also check out filter_input_array() and its cousins.

I know I personally would not use the second implementation due to the lack of legibility. At least with the first and third implementations you can visually see that a value is being changed. The second is not readily obvious. However, it is most likely the more efficient. I have used both the first and third myself, but more often use the third. Think it has to do with what mood I'm in. Hope this helps, I'm interested to hear what others might have to say :)

Answer 5

I'd go with your second approach:

foreach ($array as &$item) { $item = perform_changes_on($item); } 

Or even better:

function perform_changes_on(&$item) { // ... } foreach ($array as &$item) { perform_changes_on($item); } // ... 

Working on each item (by reference) seems the safest one, since you're not accessing the array's structure, but its single elements instead (which, I take, is what you want to do).