Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:102621
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain majkl578.cz designates 74.125.82.48 as permitted sender)
MIME-Version: 1.0
Sender: mail@majkl578.cz
In-Reply-To: <CAF+90c9RB=mMD=4WsbDJMu=iyh-SO90U7TzHrh-VGjQyCLR2eg@mail.gmail.com>
References: <CAO7eTqNUo8Oy+D7PUO7acNL05EHiXdosRCZS2agiawu6JxV5+w@mail.gmail.com>
 <CAF+90c9RB=mMD=4WsbDJMu=iyh-SO90U7TzHrh-VGjQyCLR2eg@mail.gmail.com>
Date: Thu, 5 Jul 2018 22:58:37 +0200
Message-ID: <CAO7eTqNTg3EiJnjWzMHj0B1Xfbod40Np=8D2FQ3NgSchRB4+CA@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: Michael Moravec <php.net@majkl578.cz>, PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] [RFC][Vote] iterable_to_array() and iterable_count()
From: php.net@majkl578.cz (Michael Moravec)

2018-07-05 22:35 GMT+02:00 Nikita Popov <nikita.ppv@gmail.com>:

> I voted against both functions. Jordi already explained why iterable_count()
> is questionable, so let me add a comment regarding iterable_to_array().
>
> Essentially my gripe is that I'm not convinced of the usefulness of this
> function and am concerned that it might be encouraging bad practices. In
> particular:
>
>  * As the RFC already mentions, iterable_to_array() can be replaced by a
> simple and in my eyes very clear one-liner. This pattern would have to be
> extraordinarily common to justify adding a function for it.

Now imagine a large codebase with 50 occurences. Then multiply this by 1000
projects. That is a lot of "simple oneliners" with a lot of place for
potential errors.

In the end this is not different from is_countable() which was
unanimously accepted.

The pattern is not common yet because iterable is not common yet. One of the
show-stoppers is incompatibility with array functions (or array hints
in general).

>  * The operation iterable_to_array() *should* be rare. Please do not accept
> iterable arguments if you are not just iterating over them. If your
> operation requires conversion to array, then directly accept an array,
> otherwise you are making false promises. If you want to use an array
> function on an iterable -- don't. Reimplement the function to work with
> iterables or use a library that does.

Unfortunately, this is not true in real world, mostly due to BC and
compatibility.

>  * The very close names of iterator_to_array() and iterable_to_array() are
> somewhat confusing. For type declarations "iterable" and "Iterator" are
> quite distinct due to different capitalization, but the functions are hard
> to distinguish at a glance.

I disagree, but I also believe this is a matter of taste/preference.

> An additional issue is that iterable_to_array() preserves the behavior of
> iterator_to_array() to preserve iterator keys by default, which is a big
> footgun, especially in combination with generators. However, I also
> understand that using a different default would be even worse, due to the
> confusion with iterator_to_array().

Whether it's true or not, thatt's what iterator_to_array() does, and
the behavior is neither deprecated nor adviced against in documentation.
If you want to deprecate keyed iterator_to_array(), let's do it in 7.4,
but I don't see how this makes this very RFC "bad", just by preserving
behavior for consistency.
Right now, it sounds like advocating the introduction of yet another
inconsistency, something that PHP is very often being disliked for.

Thanks,
M.