Created
October 22, 2016 15:19
-
-
Save enygma/c3fff1ceb3f6b6643ae5c7b182a81f53 to your computer and use it in GitHub Desktop.
Is this a bug with the iterator handling and foreach?
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
<?php | |
/** | |
* In the example below, I make an iterator and assign three values to it: "foo", "bar" and "baz". | |
* I then manually remove the first one (index 0) and pass the rest into foreach. I expected it | |
* to just start with index 1 and go on and loop through the rest but there's no output at all. | |
* | |
* Is this a bug? | |
*/ | |
class Items implements \Iterator | |
{ | |
private $items = []; | |
private $index = 0; | |
public function current() | |
{ | |
return $this->items[$this->index]; | |
} | |
public function next() | |
{ | |
$this->index++; | |
} | |
public function valid() | |
{ | |
return isset($this->items[$this->index]); | |
} | |
public function key() | |
{ | |
return $this->index; | |
} | |
public function rewind() | |
{ | |
$this->index = 0; | |
} | |
public function add($item) | |
{ | |
$this->items[] = $item; | |
} | |
public function remove($index) | |
{ | |
unset($this->items[$index]); | |
} | |
} | |
//------------------- | |
$items = new Items(); | |
$items->add('foo'); | |
$items->add('bar'); | |
$items->add('baz'); | |
$items->remove(0); | |
// Expected that the loop would start with the first index of "1" | |
// but it doesn't output anything | |
foreach($items as $item) { | |
echo $item."\n"; | |
} |
@enygma how about changing the unset($this->items[$index]);
to array_splice($this->items, $index, 1);
? This will reset the indices.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The bug is in the implementation, which contains two contrary assumptions:
rewind()
andnext()
(and the way you initialise$index
) assume the underlying array will have sequential numeric keys starting at0
.remove()
removes array entries but will lead to non-sequential keys in the underlying arrayThe combination of these two contradictory models breaks because as @dzuelke points out,
valid()
will fail at the first unset key. This is not a PHP issue - your object is controlling at all times what$index
is set to, and should take responsibility for how it's moved forwards or reset (iterators can of course have whatever keys they want, numeric or otherwise).You can resolve this by either making
rewind()
andnext()
more aware of what keys are actually in the array, or you can makeremove()
renumber the array when removing entries.