Re: Make sorting stable

On Wed, Mar 4, 2020 at 7:35 PM Sara Golemon <pollita@php.net> wrote: > On Wed, Mar 4, 2020 at 12:12 PM Nikita Popov <nikita.ppv@gmail.com> wrote: > >> On Wed, Mar 4, 2020 at 6:17 PM Sara Golemon <pollita@php.net> wrote: >> >>> On Wed, Mar 4, 2020 at 10:42 AM Nikita Popov <nikita.ppv@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