Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118905
MIME-Version: 1.0
References: <DM6PR14MB4155BEF0A6C924DB3812C97DF9329@DM6PR14MB4155.namprd14.prod.outlook.com>
In-Reply-To: <DM6PR14MB4155BEF0A6C924DB3812C97DF9329@DM6PR14MB4155.namprd14.prod.outlook.com>
Reply-To: Levi Morrison <levi.morrison@datadoghq.com>
Date: Fri, 28 Oct 2022 11:43:50 -0600
Message-ID: <CAAKU0umca3aY+Ha31Vr9h0ZHMYBqO9aetsS8x0KJo8iXKPsZyA@mail.gmail.com>
To: tyson andre <tysonandre775@hotmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] Proposal: Expanded iterable helper functions and
 aliasing iterator_to_array in `iterable\` namespace
From: internals@lists.php.net ("Levi Morrison via internals")

> Namespaces were chosen after feedback on my previous RFC,
> and I believe `iterable\` follows the guidance from https://wiki.php.net/rfc/namespaces_in_bundled_extensions and
> https://wiki.php.net/rfc/namespaces_in_bundled_extensions#core_standard_spl

I support this general direction :thumbsup:

> I plan to create an RFC with the following functionality in the iterable\ namespace, and wanted to see what the preference on naming was, or if there was other feedback.
> (Not having enough functionality and wanting a better idea of the overall
>
> - `iterable\count(...)` (alias of iterator_count)
> - `iterable\to_array(Traversable $iterator, bool $preserve_keys = true): array` (alias of iterator_to_array, so that users can stop using a misleading name)

At the very least, I'd like to also see `to_array_list` which ensures
that the `array_is_list` invariant is upheld by only using values from
the iterable.

Variants for assoc arrays have to deal with the fact that iterables
have duplicate keys, and also may contain key types which don't work
as array keys.

 1. I think later values overwriting previous values of the same is
generally agreeable.
 2. However, what to do with things that aren't key types I can see
having a lot of discussion. Some people probably just want them to
silently filter out, some people want a warning but to proceed with
the list, others may want a TypeError. I suppose we can make 3
functions, one for each case?
    - `to_assoc_array_silent` which drops things which don't work as array keys.
    - `to_assoc_array_warn` which drops things which don't work as
array keys but also emits a warning.
    - `to_assoc_array_throw` which throws a TypeError if a key doesn't
work as an array key.

Not sure on these assoc things, but I'm pretty sure `to_array_list`
should be included in the very first version.

> - `iterable\any(iterable $input, ?callable $callback = null): bool` - Determines whether any value of the iterable satisfies the predicate.
>    and all() - Determines whether all values of the iterable satisfies the predicate.

:thumbsup:

> - `iterable\none(iterable $input, ?callable $callback = null): bool`
>
>    returns the opposite of any()

:thumbsup:

> - `iterable\find(iterable $iterable, callable $callback, mixed $default = null): mixed`

I would prefer to drop the default and have an Option return type
instead, but we don't have one today.

> - `iterable\fold(iterable $iterable, callable $callback, mixed $initial): mixed`
>
>   `fold` and requiring an initial value seems like better practice. See https://externals.io/message/112558#112834
>   and https://stackoverflow.com/questions/25149359/difference-between-reduce-and-fold

:thumbsup:

If we figure out an optional/result type then I think
`iterable\reduce` which just uses the first item as the initial would
be very helpful, but we have to deal with the empty iterable case so
I'm happy to leave it out for now since we don't have Option.

> - `iterable\unique_values(iterable $iterable): array {}`
>
>   Returns true if this iterable includes a value identical to $value (`===`).

I suppose this should take an optional callback that users can provide
for a custom definition of uniqueness?

> - `iterable\includes_value(iterable $iterable, mixed $value): bool {}`
>    Returns a list of unique values of $iterable

Not sure on this one, but the description doesn't match the signature :)
Anything which does comparison/uniqueness check should have a callback
(or optional callback) for specifying how to do that.

> There's other functionality that I was less certain about proposing, such as `iterable\keys(iterable $iterable): array`,
> which would work similarly to array_keys but also work on Traversables (e.g. to be used with userland/internal collections, generators, etc.)

I assume this doesn't care about key uniqueness -- it takes the keys
and makes them values, in the same order they were returned?

In any case, I don't think this should return an iterable, not array,
to allow for lazy operations in a chain, while also allowing for using
an array as an optimization.

> Or functions to get the iterable\first()/last() value in an iterable. Any thoughts on those?

What do they return on empty? I would prefer to delay these and try to
get an Option type. Otherwise, they require doing a lot of variants
like the following to be ergonomic:
  - first_or($iterable, mixed $default)
  - first_or_else($iterable, callable $callback)
  - last_or($iterable, mixed $default)
  - last_or_else($iterable, callable $callback)

But if we can return an option:

  - first($iterable): Option
  - last($iterable): Option

Then the other parts move to the Option API:
  - `iterable\first($iterable)->unwrap_or($default)`
  - `iterable\first($iterable)->unwrap_or_else(fn () => $whatever)`

This general feedback also applies to `iterable\find()`.

> I also wanted to know if more verbose names such as find_value(), fold_values(), any_values(), all_values() were generally preferred before proposing this,
> since I only had feedback from a small number of names. My assumption was short names were generally preferred when possible.

I like the verbose names when there are variants, for instance `fold`
and `fold_with_keys` (where we provide both key and value). I
definitely do not like it when we change function signatures of
callbacks based on `flags` parameters like some PHP functions do
today.

Phew, I'm not sure the mailing list is the best way to the tidbits of
the APIs! In any case, I strongly support the direction.