1

I have a coworker that I noticed was using his foreachs in the following fashion:

foreach ($var as $var) {
    // do stuff here
}

I have tested the code above and it works correctly to my surprise. Would the PHP gurus like to hop in and tell me why this is wrong? It feels very wrong.

Brad
  • 159,648
  • 54
  • 349
  • 530
cha55son
  • 423
  • 3
  • 12
  • if it's working it isn't wrong. maybe (definitely) just bad style. Anyway i would like to know the answer too. – Le_Morri Apr 30 '13 at 14:32
  • But look to $var after loop. – sectus Apr 30 '13 at 14:32
  • if you mean using $var on both sides of the `as` then it is very wrong as you are overriding the value being iterated with the element of the iteration – fullybaked Apr 30 '13 at 14:32
  • I assume the value of $var is copied somewhere when the foreach starts, which would avoid having an error ? Does it work this way ? – Virus721 Apr 30 '13 at 14:34
  • @Virus721: Sort of. If you like to understand all the glory details and when that is and when not, see: [How foreach actually works](http://stackoverflow.com/q/10057671/2261774) – M8R-1jmw5r Apr 30 '13 at 14:35

5 Answers5

4

Because it changes the value of $var. After the foreach() it's no longer an array but is set to the last value in the array.

$var = array('apple', 'orange');
foreach ($var as $var) {
    echo $var."<br/>";
}
echo $var; //orange

If you don't want to change the variable's value, it'll need to be a different variable name:

$var = array('apple', 'orange');
foreach ($var as $fruit) {
    echo $fruit."<br/>";
}
echo $var; //array

As @UselessIntern pointed out, it's fine if you're not going to use the variable after looping through it, but it's definitely not encouraged because it can lead to confusion.

As @PLB pointed out, it iterates over a copy of the $var not $var itself. So every iteration the value of $var is changing, but it doesn't break the loop because it's looping over the copy that was created.

Ben
  • 60,438
  • 111
  • 314
  • 488
  • 1
    Which works if you want to read something but nasty surprises await with anything else. – Useless Intern Apr 30 '13 at 14:35
  • The first loop should (if it works in a natural way) do `$var = $var[0]`, i.e we're losing 'orange'. Why not explaining that instead ? – Virus721 Apr 30 '13 at 14:35
  • @Virus721 - I'm not sure I follow... we're not losing orange, we're losing apple. Orange is the 2nd iteration, so `$var` will be `orange`; – Ben Apr 30 '13 at 14:37
  • If we assign to $var the value of it's first element, then $var becomes it's first element, and the arrays gets lost as we no longer have a reference to it, i assume. – Virus721 Apr 30 '13 at 14:38
  • 2
    I guess main question was: `why does not foreach loop crash after first iteration because $var is not an array anymore?` This happens because foreach iterates over a copy of original array. Anyway, constructions like this should be avoided (even if they are technically correct). – Leri Apr 30 '13 at 14:38
  • This is possible because it allows to iterate over the same array at once again (`foreach($array as $a) { foreach($array as $b) { $a; $b; }}`), however this questions example is a bad example of [variable-reuse](http://stackoverflow.com/a/16302471/2261774). – M8R-1jmw5r Apr 30 '13 at 14:49
  • I have confirmed this behavior. Thanks for the insight. – cha55son Apr 30 '13 at 16:52
0

Even it feels wrong, it still works because the moment the foreach starts, PHP internally has already access on the data.

So even $var gets overwritten, in memory this data is still present (the original array) and $var is set in each iteration to the current value of it.

The concrete problem you've spotted and about which you say is wrong is also known as variable-reuse which you should prevent because this is a code-smell.

It not only feels wrong, it is wrong to write such code. Tell your co-worker so you can write better code together.

M8R-1jmw5r
  • 4,896
  • 2
  • 18
  • 26
0

Because it's a loop. Performing:

array > string
array > string

So

foreach ($var AS $var){
 /*
 Is basically being re-read at the end so your foreach can read the array again to get  the next step of the array 

 array > string 
 recheck array > string
 recheckarray > string 
 */
}
Daryl Gill
  • 5,464
  • 9
  • 36
  • 69
0

Check this expression:

$x = array(1,2,3);
foreach ($x as $x) {
    echo $x; //prints 123
}

What is happening here is that the foreach extracts the first element of array $x and overrides into $x itself. However the array variable $x which was on the left side of the as keyword remains in internal scope of the foreach argument which is why the looping works without problems.

Once the loop completes the $x which was internal to the foreach (the array) loses scope and no longer exists, and what remains is the $x variable which now contains the last element of the original $x array. In this case that would be 3.

raidenace
  • 12,789
  • 1
  • 32
  • 35
0

Your coworker seems to don't need $var as an array after the loop anymore. When PHP initializes the foreach loop (what is done only once) it uses the original values from $var as it is an array at the moment. Then on every step in the loop the current element of the element is assigned to a new var called var. Note that the orginal array $var doesn't exist anymore. After the loop $var will have the value of the last element in the original array.

check this little example which demonstrates what I've said:

$a = array(1,2,3);
foreach($a as $a) {
    echo var_dump($a);
}

// after loop
var_dump($a); // integer(3) not array

I could imaging that your coworker does this to save a little memory as the reference to the array will get overwritten and therefore the garbage collector will remove it's memory on next run, but I would not advice you to do the same, as it's less readable.

Just do the following, which is the same but is much more readable:

$array = array(1,2,3);
foreach($array as $value) {
    echo var_dump($value);
}

delete($array);
delete($value);
hek2mgl
  • 152,036
  • 28
  • 249
  • 266