1

I'm trying to get better understanding of iterators in PHP. For this test I wanted to make a tree of items, and list them with different RecursiveIteratorIterator modes. ::SELF_FIRST and ::CHILD_FIRST modes are working as I expect them to. However, it doesn't when I want to list leaves. There must be something I'm missing in implementation which doesn't allow that mode to work properly as it prints out nothing. Is there something wrong with my Obj::hasChildren() method?

Here is the test class:

class Obj implements \RecursiveIterator {
    public $children = array();

    private $position;

    private $name;

    public function __construct($name)
    {
        $this->name = $name;
    }

    public function valid()
    {
        return isset($this->children[$this->position]);
    }

    public function next()
    {
        $this->position++;
    }

    public function current()
    {
        return $this->children[$this->position];
    }

    public function rewind()
    {
        $this->position = 0;
    }

    public function key()
    {
        return $this->position;
    }

    public function hasChildren()
    {
        return !empty($this->children[$this->position]);
    }

    public function getChildren()
    {
        return $this->children[$this->position];
    }

    public function __toString()
    {
        return $this->name;
    }
}

And here is the test:

use RecursiveIteratorIterator as RII;

$o1 = new Obj('Root');

$i1 = new Obj('Item 1');
$i12 = new Obj('Subitem 2');
$i1->children[] = new Obj('Subitem 1');
$i1->children[] = $i12;

$i12->children[] = new Obj('Subsubitem 1');
$i12->children[] = new Obj('Enough....');

$o1->children[] = $i1;
$o1->children[] = new Obj('Item 2');
$o1->children[] = new Obj('Item 3');

foreach (new RII($o1, RII::LEAVES_ONLY) as $o) {
    echo "<br>" . $o;
}
hakre
  • 193,403
  • 52
  • 435
  • 836

1 Answers1

2

What you assume is pointing in the right direction, there is a problem with the hasChildren() method you have. Compare it with the valid() and with the current() method, you then probably already see that it will always return true.

Because as long as there is a current(), hasChildren() returns true:

public function current()
{
    return $this->children[$this->position];
}

and:

public function hasChildren()
{
    return !empty($this->children[$this->position]);
}

Instead you want to test if the current element has children or not:

public function hasChildren()
{
    return !empty($this->current()->children);
}

The slight difference that will give you your output:

Subitem 1
Subsubitem 1
Enough....
Item 2
Item 3

By always returning TRUE for hasChildren(), the RecursiveIteratorIterator is unable to detect any leaves. By concept of a tree, this is not possible but in a traversal process - as you demonstrated with your "bug" - clearly possible :)

See as well (if I may):

Community
  • 1
  • 1
hakre
  • 193,403
  • 52
  • 435
  • 836
  • You're absolutely right! Thanks for this! And amazing in-depth RII explanation, this is far better than official documentation! –  Jul 26 '13 at 20:28
  • Also, I might have some weird way of reasoning, but I don't find methods hasChildren() and getChildren() properly named? If I have a class which implements RecursiveIterator interface, I basically have to rename my getChildren() (which would return all children as array or something similar) to something less descriptive. –  Jul 26 '13 at 20:33
  • @igorpan: For the naming: Take in mind that the Iterator object must not be your collection object. It's just the iterator object that knows how to iterate over your collection. So you don't have to do that in one object. You see the difference? (If not just ask back) – hakre Jul 27 '13 at 14:27
  • I realise that, but since my collection is a plain&simple array, would it really make any sense to use an object for that purpose? –  Jul 28 '13 at 14:19
  • Yes it does, because the iterator adds the recursive iteration for your specific array "type" - it implements for example the way you line-up the childnodes in the array. So it makes especially sense because you can keep the array as container structure and for the iteration only you create the iterator. That way you can benefit from the recursive iteration over your array and in case it changes, you create a new iterator. The rest of the code does not need to change. – hakre Jul 28 '13 at 14:40