Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113112
MIME-Version: 1.0
References: <CAFMT4Np4JCB+cSEdEWKXWXUX83fq0kt+P6F-F=LvfktpjLf=3w@mail.gmail.com>
 <CAF+90c_g9+4UEsBshZ-G_KUTgYbJZNAS597JDD-FrfZPwTsW4g@mail.gmail.com>
In-Reply-To: <CAF+90c_g9+4UEsBshZ-G_KUTgYbJZNAS597JDD-FrfZPwTsW4g@mail.gmail.com>
Reply-To: Levi Morrison <levi.morrison@datadoghq.com>
Date: Mon, 8 Feb 2021 08:24:17 -0700
Message-ID: <CAAKU0umnsFRfN5cHXh9Rn77b=DCZuyPCo8+Ziu+DuY5yoouL4g@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: Levi Morrison <levim@php.net>, internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] Proposal: Add ReverseArrayIterator and
 ForwardArrayIterator to SPL
From: internals@lists.php.net ("Levi Morrison via internals")

On Wed, Feb 3, 2021 at 8:50 AM Nikita Popov <nikita.ppv@gmail.com> wrote:
>
> On Wed, Feb 3, 2021 at 4:38 PM Levi Morrison <levim@php.net> wrote:
>
> > Hello, everyone!
> >
> > This proposal adds two new classes to the SPL:
> >
> >   - `Spl\ReverseArrayIterator`. It iterates over an array in reverse
> > order. It does not duplicate the array.
> >   - `Spl\ForwardArrayIterator`. It iterates over an array in forward
> > (normal) order. It does not duplicate the array.
> >
> > They both implement Countable which returns the `count()` of the
> > array. The [PR][1] has some examples and discusses why I am proposing
> > `ForwardArrayIterator` when there is already `ArrayIterator`, the
> > short of which is for performance. There are timing numbers in [one of
> > the comments][2].
> >
> > When it comes time to vote I may merge this into another RFC with
> > [`CachedIterable` by Tyson Andre][3], which I recommend readers also
> > take a look at. Whether we team up for the RFC vote or not, I wanted
> > to get this out there for discussion and review.
> >
> >   [1]: https://github.com/php/php-src/pull/6535
> >   [2]: https://github.com/php/php-src/pull/6535#issuecomment-769179450
> >   [3]: https://github.com/php/php-src/pull/6655
> >
>
> Hey Levi,
>
> I like the general idea of having an "ArrayIterator but sane".
>
> That said, I don't think that the ReverseArrayIterator +
> ForwardArrayIterator pair of iterators approaches this problem correctly.
> There are plenty of iterators that could be run in reverse, and I think it
> would be silly to create two classes for each of them. E.g. if we introduce
> an ObjectPropertyIterator, should there be both
> ForwardObjectPropertyIterator and ReverseObjectPropertyIterator? I don't
> think so.
>
> I think the correct abstraction for bidirectional iterators is to introduce
> an interface
>
> // Or "ReversibleIterator"
> interface BidrectionalIterator extends Iterator {
>     public function prev(): void;
>     public function end(): void;
> }
>
> and then a class along the lines of:
>
> class ReverseIterator implements BidirectionalIterator {
>     public function __construct(private BidirectionalIterator $iter) {}
>
>     public function next() { $this->iter->prev(); }
>     // etc.
> }
>
> This would replace "new ReverseArrayIterator($array)" with "new
> ReverseIterator(new ArrayIterator($array))", but in a way that is general,
> and composes.
>
> Regards,
> Nikita

After thinking about it for a while, I think this will be fine. It
doesn't encroach on the reversible IteratorAggregate space, which is
the space that I didn't like from my previous proof-of-concept.

I'd like it if `BidirectionalArrayIterator` was shorter, but plain
`\Spl\ArrayIterator` is too likely to be confusing with
`\ArrayIterator` and that's the only meaningfully shorter name I've
thought of yet. This also plays into the bad ergonomics of the
reversed array iterator:
    new ReverseIterator(new BidirectionalArrayIterator($array))

But as you say, the reversed version will not be common.

I'll code this up soon unless someone objects. I'm very open to naming
suggestions for a better performing, simpler array iterator.