Lies! Darn Lies!

Been talking to Marcus Boerger, via email, about the PHP 5 Standard Library article and need to confess ;)

Marcus makes a very good point about the DirectoryReader example I used;

Your DirectoryReader is wrong. It does the non file skipping in valid, but it should be done in next().

But after the necessary changes it still has a major design flaw. It uses recursion. And Iterators are about unfolding.

So what you should do is preventing that recursion and if you look at the result and how much it took you to complete it correctly you see why using FilterIterator would have been better. That one can be used to filter out the non files easily and it makes clear why iterators can be used for functional programming – in this scenario the filter algorithm provided by FilterIterator doesn’t know anything about your data.

Here’s the problem class plus code using it;


class DirectoryReader extends DirectoryIterator {

function __construct($path) {
parent::__construct($path);
}

function current() {
return parent::getFileName();
}

function valid() {
if ( parent::valid() ) {
if ( !parent::isFile() ) {
parent::next();
return $this->valid();
}
return TRUE;
}
return FALSE;
}

function rewind() {
parent::rewind();
}
}

// Create a directory reader for the current directory
$Reader = new DirectoryReader('./');

// Loop through the files in the directory ?!?
foreach ( $Reader as $Item ) {
echo $Item.'
';
}

Anyone want to take up the guantlet and re-implement?

As I understand what Marcus is saying, the alternative is to use DirectoryIterator itself then pass it to something extending FilterIterator (or perhaps better extending DirectoryFilterDots) – the “spec” is for something to list only the files in a given directory, ignoring the parent and current nodes.

Win an Annual Membership to Learnable,

SitePoint's Learning Platform

  • Ren

    I’d go for the shortest method


    class FileFilterIterator extends FilterIterator
    {
    function accept() { return $this->current()->isFile(); }
    }
    $Reader = new FileFilterIterator(new DirectoryIterator('./'));

    // Loop through the files in the directory ?!?
    foreach ( $Reader as $Item ) {
    echo $Item.'
    ';

    As a side note, been wondering why an abstract OuterIterator class wasn’t created in spl.


    abstract class OuterIterator implements Iterator
    {
    protected $iterator;

    function __construct(Iterator $iterator) { $this->iterator = $iterator; }
    function getInnerIterator() { return $this->iterator; }
    function valid() { return $this->iterator->valid(); }
    function key() { return $this->iterator->key(); }
    function current() { return $this->iterator->current(); }
    function next() { return $this->iterator->next(); }
    function rewind() { return $this->iterator->rewind(); }
    }

    which then iterators like FilterIterator would have been subclasses of.

  • http://www.phppatterns.com HarryF

    Re the original title – to keep life simple have changed it; not interested in the discussion that surrounds it and this is the shortest path this time.

    Cant guarantee Ill do the same in future – to me, as a European, there was nothing offensive about the title – didnt even occur to me. If I start thinking about every possible culture I might offend, nothing would get written.

    As a side note, been wondering why an abstract OuterIterator class wasn’t created in spl.

    Thats an interesting question – guess we’d need Marcuss input on that. Talking to him and studying the design I’m very sure he had clear ideas in building the SPL iterators; the problem is I doubt he’s going to get time to explain all the decisions that influenced the design, which would help alot.

    Perhaps you could propose it to him, with a use case?

  • http://www.mission36teen.com M36Teen

    Thanks Harry, keep up the PHP stuff, I’ve learned a LOT from you! :-)

  • beatybeaty

    I too, would like to thank you, Harry. Keep up the great work. I’ve learned a lot from you as well, including your books. (a *damn* lot) ;) ;)

  • spags

    Love your postings Harry! thanks, great job

  • http://www.phppatterns.com HarryF

    Ren – message from Marcus which I think was meant to endup here;

    Your idea is not bad but you lost the subdirs. You need to use
    RecursiveDirectoryIterator instead of DirectoryIterator. Then you
    need to iterate that with RecursiveIteratorIterator and that with
    a filter iterator. The last being something like the opposite of
    ParentIterator or one that uses your accept().

    The outeriterator is an idea both Andi and me had. However it got
    lost somewhen because i didn’t fint it to important and i would
    not allow access to the inner iterator by property. Instead i
    would make that a private one and provide a function based
    access. Also it doesn’t need to be abstract in your case but i
    want that so i can still have c implementations. As a consequence
    i’d do it in an Interface. Putting those ideas together gives teh
    following:

    interface OuterIterator extends Iterator
    {
    abstract function getInnerIterator();
    }

    and then maybe have a class IteratorIterator like your one which
    implements OuterIterator :-)

  • Ren

    Ah yes, I didnt intend for getInnerIterator to be public.

    I’m finding the general class helpful thou, especially when mapping either key() or current() return values to something else.

  • Alexios Fakos

    How about that?:)


    //set_time_limit(10);

    class DirectoryReader extends FilterIterator {

    protected $iter;

    function __construct($path) {

    $this->iter = new RecursiveIteratorIterator (
    new RecursiveDirectoryIterator($path)
    );
    parent::__construct( $this->iter );
    }

    function accept() {
    return $this->iter->getSubIterator()->isFile();
    }
    }

    $path = dirname(__FILE__);

    foreach(new DirectoryReader($path) as $file) {
    echo $file->getPath(), ' --> ', $file->getFilename(), "n";
    /*
    or invoke __toString()...
    echo $file, "n";
    */
    }