Yesterday I was debugging a legacy PHP app, and came across this item of code. This example is substantially condensed for brevity:

foreach ( $tree as $key => &$branch ) {
    if ( isset( $tree[ $key+1 ] ) && isset( $branch['node'] ) ) {
        $tree[ $key+1 ][ 'selectId' ] = $branch['node']->Data->code;
        $tree[ $key+1 ][ 'children' ][ $branch['node']->Data->code ]['selected'] = true;
    }
}

# Menu should appear in reverse
$tree = array_reverse( $tree );

# OK, the bug is exhibited here
foreach ( $tree as $branch ) {
    echo 'Checksum: ' . sha1( print_r( $tree, true ) ) . '<br/>';
}

OK, here’s the output:

Checksum: 3ca7d13b26a3382c901fdd0d3fa606439167e18d
Checksum: 14279be5bd764b559922a1f6f65d81dd78c1e7d4
Checksum: f8b38a67d18dee92e25a0e0896e8f1a526c2915b

On first examination, this doesn’t look right at all; after all, iterating over an array with foreach should not modify the array. I discovered, looking at the output of print_r(), that it was the first element of $tree that was being modified upon each iteration.

(The answer comes after this paragraph, so developers wanting to work it out should stop reading :)).

As it turns out, the bug is not in the second loop; it’s in the first one. The variable $branch has been set up as a reference instead of the usual copy, using the ampersand operator — and a quick look inside that loop shows that this is unnecessary. Foreach loops will normally leave the element variable containing the value of the last element, but in this case, $branch is left as an active pointer to the last element. This pointer even survives array_reverse(), which results finally in $branch pointing to the first element of $tree.

Hence, when the second loop executes, every time $branch is written to, it overwrites the first element of the array. To fix this, it would have been sufficient to unset($branch) after the first loop, or better yet, to avoid the reference operator entirely.