ARRAY_UNIQUE_IDENTICAL option

On 3 November 2022 18:53:40 GMT, someniatko <someniatko@gmail.com> wrote:

Actually, I think this is already the case for "normal" objects - I had no 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/UGCvB Making backed enums work with their backing value would be equally confusing 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 discarded. Regards,

To avoid noise I will respond to all e-mails at once. --- Hi someniatko

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

The default string strategy is not great. Unfortunately, changing this is likely not an option as there is no clear migration path.

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

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.

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

Hi Levi Morrison, > A bit off topic, but not entirely: > > 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); > > Of course, there are other things like instead of using int for `0`, > `-1`, `1`, we could have used an enum but we don't have one today. I > just mean the core idea of taking callable is better than mucking > around with flags while also allowing for custom comparison. Note that > it doesn't necessarily prevent optimizations either. For instance, if > they had passed `php_compare` or some function which represents `$a > <=> $b`, we could identify that just as we identify a specific flag > and take an optimized pass. 1. Calling php functions from C is fairly slow. Sorting compared to hash maps is also slow If we did want a specialized implementation for user-provided equality criteria, `?callable(T $a): U $iteratee = null` would seems more practical to me (Calling a function `n` times rather than `n log n` times would be faster, and this would work even for cases such as `[$obj->nonObjectField1, $obj->field2->toNormalizedRepresentation()]` instead of using a comparator in most cases (by putting both arrays into the internal hash map) (same approach as https://lodash.com/docs/#uniqBy) 2. `<=>` isn't a stable order for int/string/float (etc) in various ways, so some comparators would have issues and return duplicates. It's possible to implement stable comparisons, but I don't really expect enthusiasm for that https://github.com/TysonAndre/pecl-teds/#stable-comparison 3. I expect a majority of cases could use the `ARRAY_UNIQUE_IDENTICAL` directly. (or use that on array_map()ped values to find the keys of the original array to use) So requiring the use of $comparator would result in longer code and more possible sources of bugs (e.g. a comparator returning `$a - $b` might overflow/underflow int if users don't realize `<=>` should be used) Thanks, Tyson