Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113129
MIME-Version: 1.0
References: <DM6PR07MB66189594D6D3B6D820E4A5FBF98F9@DM6PR07MB6618.namprd07.prod.outlook.com>
 <77dfe9e5-a6d1-4a41-bceb-454a65cf34d0@www.fastmail.com> <DM6PR07MB6618999C8F1AAC35BF2F8C87F98F9@DM6PR07MB6618.namprd07.prod.outlook.com>
 <f1abc6fd-dfad-4cd4-a124-58475312fa86@www.fastmail.com> <DM6PR07MB66184FDB329A71B371883C54F98E9@DM6PR07MB6618.namprd07.prod.outlook.com>
 <CAAKU0u=5ypeaT7u3xi-zT=cXLJxJYXrL_LLaV+HRXHmYSJWwOA@mail.gmail.com> <5ed1c099-6a0b-4e92-96a7-a1f21aefe753@www.fastmail.com>
In-Reply-To: <5ed1c099-6a0b-4e92-96a7-a1f21aefe753@www.fastmail.com>
Date: Wed, 10 Feb 2021 09:54:53 +0100
Message-ID: <CAC5V6gjrQeba5uMW00GC11fW1SbHiQkYU6GsFyz4bVsZ57RVLw@mail.gmail.com>
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>, tyson andre <tysonandre775@hotmail.com>
Content-Type: multipart/alternative; boundary="0000000000001d4a0105baf78fe7"
Subject: Re: [PHP-DEV] [VOTE] PHP\iterable\any() and all() on iterables
From: guilliam.xavier@gmail.com (Guilliam Xavier)

--0000000000001d4a0105baf78fe7
Content-Type: text/plain; charset="UTF-8"

Hi,

On Tue, Feb 9, 2021 at 7:33 PM Larry Garfield <larry@garfieldtech.com>
wrote:

> On Mon, Feb 8, 2021, at 6:48 PM, Levi Morrison via internals wrote:
> > On Mon, Feb 8, 2021 at 5:15 PM tyson andre <tysonandre775@hotmail.com>
> wrote:
> > >
> > > Hi Larry Garfield,
> > >
> > > > > Hi Larry Garfield,
> > > > >
> > > > > > > Hi internals,
> > > > > > >
> > > > > > > Voting has started on
> https://wiki.php.net/rfc/any_all_on_iterable and
> > > > > > > ends on 2021-02-22.
> > > > > > >
> > > > > > > This RFC proposes to add the functions
> `PHP\iterable\any(iterable
> > > > > > > $input, ?callable $callback = null): bool` and
> `PHP\iterable\all(...)`
> > > > > > > to PHP's standard library's function set, using the namespace
> preferred
> > > > > > > in the previous straw poll.
> > > > > > >
> > > > > > > [...] <https://www.php.net/unsub.php>
> > > > > >
> > > > > >
> > > > > > Ak!  I literally just finished reading it and wanted to note a
> lack of clarity on one point. :-)
> > > > > >
> > > > > > The signature of the callback is never specified explicitly.
> The ternary is a bit confusing.  I assume the signature is
> > > > > >
> > > > > > callable(mixed): bool
> > > > > >
> > > > > > But that's not made explicit.  It's also not made explict that
> omitting the callable collapses to "is truthy".  That's a sensible thing to
> do, but it's not stated explicitly anywhere, just inferred from the code
> sample.
> > > > > >
> > > > > >  [...]
> > > > >
> > > > > If there is a callable, it allows `callable(mixed): mixed`,
> > > > > and converts the callable's return value to a boolean.
> > > > > So omitting the callable is the same as passing in the callable
> `fn($x)
> > > > > => $x`, which is equivalent to `fn($x) => (bool)$x`.
> > > > > This is exactly what the reference implementation would do.
> > > > >
> > > > >  [...]
> > > >
> > > > Oof.  I'm glad I asked, because I don't like that at all.  If
> available, the callable should be returning bool, not "anything that may be
> truthy/falsy."  If you have an explicit function, it should have an
> explicit return type.  A truthy check is a reasonable default, but not for
> when you're opting in to specifying the logic.
> > > >
> > > > I am in favor of the RFC, but I will have to consider if that
> changes my vote to No.
> > > >
> > > > --Larry Garfield
> > >
> > > This was a deliberate choice and is consistent with the weak type
> comparison behavior of array_filter() and other functions that default to
> using weak type checks internally.
> > >
> > > I'd agree that I'd prefer to see callbacks returning booleans in code
> I'm reviewing,
> > > but a truthiness check seems more practical and consistent with the
> rest of the language
> > > than throwing a TypeError or checking the predicate return value using
> `!== true`
> > >
> > > This was made to make PHP more widely accessible and free of surprises.
> > > e.g. `(bool)array_filter($arr, $predicate)` can be safely converted to
> `any($arr, $predicate)` without introducing a TypeError or behavior change.
> > >
> > >  [...]
> >
> > For what it is worth, in C++ it is fairly normal to use a convertible
> > to bool type. For instance, having an overload on a < b for iterators
> > can return whatever type it wants, as long as it is contextually
> > convertible to bool.
>
> Yet in 8.0, a non-[ 1 | 0 | -1 ] return from a comparison function as just
> converted to a warning.  So the trend in the language seems to be the other
> direction.
>

Are you referring to this frequent mistake?

```
$list = [2,4,1,3];
usort($list, fn ($a, $b) => $a < $b); /* Deprecated: usort(): Returning
bool from comparison function is deprecated, return an integer less than,
equal to, or greater than zero */
echo json_encode($list); // [4,3,2,1]
```

(That's probably because PHP user comparison functions were modelled from C
(e.g. strcmp()), not C++ (e.g. std::less).)

Of course here the "correct" thing is to return `$a <=> $b` (or `$b <=> $a`
for descending order), but you can also return `$a - $b` (not necessarily
in [-1,0,1]), or even a string `"foo"` still without any warning in 8.0.2
(just a certainly wrong result)...

Anyway, to me it feels natural that any()/all() would "work" like
array_filter().

@Tyson by the way, in the any()/all() case (vs the any_value()/all_values()
and potential any_key()/all_keys() etc.), wouldn't it be preferable to add
the optional `int $flags = 0` (or "$mode") parameter right from the start
(even if not used yet), as adding it in a later release would apparently
pose some BC concerns (ArgumentCountError, polyfills etc.)?

-- 
Guilliam Xavier

--0000000000001d4a0105baf78fe7--