On Wed, Mar 4, 2020 at 7:35 PM Sara Golemon <poll...@php.net> wrote:
> On Wed, Mar 4, 2020 at 12:12 PM Nikita Popov <nikita....@gmail.com> wrote:
>
>> On Wed, Mar 4, 2020 at 6:17 PM Sara Golemon <poll...@php.net> wrote:
>>
>>> On Wed, Mar 4, 2020 at 10:42 AM Nikita Popov <nikita....@gmail.com>
>>> wrote:
>>>
>>>> The only issue I ran into is that this change has a negative impact on
>>>> code
>>>> using illegal comparison callbacks like this:
>>>>
>>>> usort($array, function($a, $b) {
>>>> return $a > $b;
>>>> });
>>>>
>>>> Let's define what "negative impact" means in this regard. Is it that
>>> one still winds up with an essentially sorted array, but hitherto "stable
>>> appering" output is now stable in a different way? Or is the result
>>> actually just NOT sorted in a way that a reasonable user would consider
>>> correct (e.g. 5 sorted before "3")?
>>>
>>
>> "Negative impact" is PR speak for "completely broken" ;) Yes, it could
>> sort 5 before 3. The comparison function becomes completely ill-defined and
>> you get Garbage-In-Garbage-Out.
>>
>> Roger that, thanks for explaining. 👍
>
>
>> If we are concerned about this (I'm not sure we should be, but I've at
>> least seen people be interested about this peculiar behavior:
>> https://stackoverflow.com/questions/59908195/how-come-usort-php-works-even-when-not-returning-integers),
>> there's two things we can do:
>>
>> 1. As Tyson suggests, we can throw a warning if a boolean is returned
>> from the comparison callback (probably with a check to only throw it once
>> per sort), to make it obvious what the cause is.
>>
>> 2. We can fix this transparently by doing something like this internally:
>>
>> $result = $compare($a, $b);
>> if ($result !== false) {
>> return (int) $result; // Normal behavior
>> }
>>
>> // Bad comparison function, try the reverse order as well
>> return -$compare($b, $a);
>>
>>
> ¿Por que no los dos?
>
> Fixing broken stuff transparently for a smooth upgrade path PLUS letting
> people know they're doing it wrong seems win-win and low cost.
>
I've implemented this variant now. If the comparison function returns a
boolean, you get
> Deprecated: usort(): Returning bool from comparison function is
deprecated, return one of -1, 0 or 1 instead in %s on line %d
once per usort() call, and we retry with swapped operands to ensure
compatibility.
Regards,
Nikita
