Is this a bug with the iterator handling and foreach?

iterator-error.php

<?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";
}

I ran it through 3v4l.org too: https://3v4l.org/UXF3A

valid() will return false so even the first iteration never happens!

@dzuelke I guess I'll just have to get the values directly then....bleh.

The bug is in the implementation, which contains two contrary assumptions:

1. The implementations of rewind() and next() (and the way you initialise $index) assume the underlying array will have sequential numeric keys starting at 0.
2. The implementation of remove() removes array entries but will lead to non-sequential keys in the underlying array

The 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() and next() more aware of what keys are actually in the array, or you can make remove() renumber the array when removing entries.

@enygma how about changing the unset($this->items[$index]); to array_splice($this->items, $index, 1);? This will reset the indices.