Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118982
MIME-Version: 1.0
References: <CAPyj-LCy-fiXuVSDb8tM+Nifv+9xk=1KGDvk-_aHdkQu7j7ADA@mail.gmail.com>
 <CAAKU0u=8nSgg8X7R26oWbgtVBHx3_gg+c4_KMpouhotB89ntWg@mail.gmail.com>
In-Reply-To: <CAAKU0u=8nSgg8X7R26oWbgtVBHx3_gg+c4_KMpouhotB89ntWg@mail.gmail.com>
Date: Mon, 7 Nov 2022 23:41:51 +0100
Message-ID: <CAPyj-LCj0W0G-Q8PS+ApvHNDBHU2m+7dAfUzGjryShDC0ZDu9A@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] ARRAY_UNIQUE_IDENTICAL option
From: tovilo.ilija@gmail.com (Ilija Tovilo)

To avoid noise I will respond to all e-mails at once.

---

Hi someniatko

> Perhaps an alternative idea is to provide a default string value for
> enums which are not baked,
> Nikolas had already brought up this idea earlier.

As others have mentioned, this opens the door for type coercion issues
and only really works for string enums. Int enums don't have an
obvious string representation. Using the name as a string value is not
intuitive (and would be really confusing when passing int enums to
string parameters) and using the stringified backed int value would
lead to collisions for most enums, as they are most likely to be
continuous sequences starting from 0.

---

Hi Rowan

> Actually, I think this is already the case for "normal" objects - I had n=
o idea that array_unique used a string cast to compare objects, so am very =
surprised that it will not consider objects of completely different classes=
 unique, if they happen to have the same string value: https://3v4l.org/UGC=
vB

The default string strategy is not great. Unfortunately, changing this
is likely not an option as there is no clear migration path.

> Making backed enums work with their backing value would be equally confus=
ing to me - Day::MONDAY and Month::JANUARY might both be backed by a 1, but=
 they are certainly distinct values. I'd much rather get an error that made=
 me check the manual and find a flag than have one of them silently discard=
ed.

I agree that a warning/error when comparing incompatible values would
be optimal. This is however off-topic and would requrie an RFC, which
I was originally hoping to avoid here.

---

HI Levi

> In my opinion, adding another flag isn't the _real_ fix. Any function
> which does comparisons should take a callable for users to provide any
> comparison they wish. An iteratively better API would be:
>
>     function array_unique<T>(list<T> $array, callable(T $a, T $b): int
> $comparator);

IMO the fact that the array is sorted (in some cases, not all cases)
is an implementation detail. The identity implementation proposed here
does not sort the array but uses a hashmap internally. A php_compare
function most likely isn't going to be useful outside of array_unique
in which case it likely shouldn't be generally available, especially
if it isn't even used internally but optimized out.

> Of course, this complaining doesn't fix the situation we are in. My
> first impression is that might be better to provide one or more
> alternative functions and to deprecate `array_unique`.

The question is if adding this option makes array_unique worse. I
don't think so. I agree that a dedicated new array_unique with saner
default values might be a good idea, but these two options are
mutually exclusive.

I'm not convinced that providing a version that accepts a closure is
necessary. Nowadays the vast majority of cases should use identity
semantics. Removing duplicates with different comparison strategies
leads to randomness. For ['1e3', 1000] both '1e3' and 1000 are valid
candidates for removal. It would make more sense to canonicalize the
array values, like converting them to integers in this case, before
then using identity semantics.

---

Anyway, as the feedback wasn't completely unanimous I will create an
RFC for this proposal.

Ilija