Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

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.

share|improve this question

5 Answers

up vote 1 down vote accepted

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.

share|improve this answer
"foreach ($array as $k => $v) is a shorthand for foreach (array_keys($array) as $k) $v = &$array[$k]" On a very picky note, that's not a shorthand as much as a difference in calling a function and a language construct (though for practical reasons, I suppose it's a shorthand). Also, it would be shorthand for a copy, not a reference. (Also, PHP is copy-on-write, so you can typically ignore extra copies.) – Corbin Jun 20 '12 at 1:46
Obviously that wanted to be foreach ($array as $k => &$v) edited and fixed. – chx Jun 20 '12 at 5:46
Would like to point out that if(!$k) is TRUE for 0, otherwise this statement is always false. So essentially you are overwriting the second array element before ever seeing it. – mseancole Jun 20 '12 at 13:31
I do that and by doing that, I am demonstrating that the first foreach works from a copy and the second doesn't. – chx Jun 20 '12 at 16:39
@chx You might want to run your first snippet then. $item is a copy of a value from $array. $array is never copied (though all elements of $array will be copied if the foreach loop completes). The first snippet will dump foo and baz just like the second one. – Corbin Jun 22 '12 at 3:33
show 2 more comments
up vote 6 down vote

This sounds like a place for array_map().

$array = array_map('perform_changes_on', $array);
share|improve this answer
+1 for using a great function that's oft-forgot. – msanford Jun 18 '12 at 15:33
indeed +1 and a cookie for you :) – mseancole Jun 18 '12 at 16:33
nice, but sometimes i won't change every item in the array. I just tried to make the example simpler... – barraponto Jun 18 '12 at 17:55
1  
perfom_changes_on doesn't have to change every element. Note that there are other array functions, some more powerful than others. array_reduce can help a lot, for example. – Quentin Pradet Jun 19 '12 at 11:46
up vote 1 down vote

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:

!!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 :)

share|improve this answer
"Arguments given to foreach are declared on each iteration, so you are technically calling array_keys() every time foreach is looped." Nope, it will only call array_keys once in this context. (Perhaps before PHP5 it had this behavior, but I do not think so as this would very easily end up in infinite loops.) – Corbin Jun 18 '12 at 17:11
@Corbin: Nope, still an issue. Don't believe me, run some tests on it and you'll see for yourself. PHP creates an iterator object for the first parameter each time it loops so that it can perform each() on each iteration of it. It is recreated, but the pointer stays the same. That's why if you changed the value of the array by reference it would be immediately available upon the next iteration :) – mseancole Jun 18 '12 at 18:09
Can you provide an example? Just tried to get it to call array_keys (well, f) twice, and could never get it to. – Corbin Jun 18 '12 at 18:20
Potentially relevant: stackoverflow.com/questions/4043569/… – Corbin Jun 18 '12 at 18:35
@Corbin: I'm honestly not sure then. I've seen someone else claiming the exact opposite, and my speed tests seem to prove it. The foreach with the "recreated" array always seems to take longer. Ran a test of over a hundred iterations and all seem conclusive that something extra is being performed in the background. I'm still looking around for that post, hopefully I can find it. I need to start keeping a repository of these posts... – mseancole Jun 18 '12 at 19:17
show 9 more comments
up vote 0 down vote

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

share|improve this answer
up vote 0 down vote

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.

share|improve this answer
thanks for the answer, I'm using array_keys to avoid the trap, but do I need to use it? I mean, foreach ($array as $key => $item) should let me change $array[$key] value, but who knows how php works internally... that's the question, actually. – barraponto Jun 19 '12 at 17:02

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.