Add `IterableExtension.sortedByDescending` to `package:collection`

[loic-sharma]

Problem

To sort GitHubIssues by their updatedAt field, descending:

issues.sortedByCompare((issue) => issue.updatedAt, (a, b) => b.compareTo(a));

This is inconvenient.

Solution

Introduce a sortedByDescending helper:

issues.sortedByDescending((issue) => issue.updatedAt);

[lrhn]

Don't want to have every version of sorting also in the reverse version.
I'd rather consider adding:

class Comparable<T> { 
  // ...
  
  static int Function(T, T) compareInverse<T extends Comparable<T>>(T v1, T v2) => v2.compareTo(v1);
}

so you can just do:

  issued.sortedByCompare((issue) => issue.updatedAt, Comparable.compareInverse);

and not have to write the compare function, just a (longer) name.

[loic-sharma]

Don't want to have every version of sorting also in the reverse version.

Could you expand why not?

so you can just do:

  issued.sortedByCompare((issue) => issue.updatedAt, Comparable.compareInverse);

This seems like it'd be useful too. However, this solution is more verbose and harder to discover.

[lrhn]

I'm just of the opinion that having a shorthand for every recognizable combination of basic operations leads to a cluttered API. It's a never-ending process to add more and more specialized variants, a lot of which see little use and don't really carry their own weight.
The more specialized a function is, the more likely that what it does is not precisely what someone else wants, so to have to add yet another variant, which is subtly different, to support the other use-case too. And then you have two almost identically named and subtly different functions in your API, which makes it more error prone for users.

Giving an efficient version of the general functionality, and letting people combine efficient primitives into the combination they need, is a better level of abstraction.
Or at least the level of abstraction that the Dart packages have so far aimed for.

That's just, like, my opinion. Maybe it's a philosophy born from having a very small team to build and support these libraries originally. (And there is still a limited amount of resources for external libraries).

[loic-sharma]

And then you have two almost identically named and subtly different functions in your API, which makes it more error prone for users.

In a previous life, I worked on .NET which has .OrderBy and .OrderByDescending.

My experience is that these APIs are not at all error prone. I can't recall a single instance where .OrderBy or .OrderByDescending were used incorrectly.

a lot of which see little use and don't really carry their own weight.

GitHub search indicates heavy use for both of these .NET APIs: 963K results for .OrderBy and 358K results for .OrderByDescending.

Giving an efficient version of the general functionality, and letting people combine efficient primitives into the combination they need, is a better level of abstraction.

I agree exposing more primitives that can be combined is a good idea. I'm not opposed to doing both :)

However, if a newbie wants to sort a list in reverse, how would they discover .sortedByCompare(..., Comparable.compareInverse)? I suspect they will need to go read docs to discover this, which would be frustrating.

On the other hand, I suspect discovering .sortedByDescending would be significantly easier.

[rakudrama]

We have four variants of sorted: IterableExtension.sorted, IterableExtension.sortedBy, IterableExtension.sortedByCompare and IterableComparableExtension.sorted.

Ascending/Descending direction is a common customization of sorting. It should no be so hard.

I agree that adding four new method names is not a great idea.
I think each of these methods should support a named optional argument descending = false.
Then the direction can become programmable - data driven - at some higher level.

Comparable.compareInverse is half a solution. It works for compareTo and not a custom comparator. If you are ordering on compareTo, you probably called one of the methods where that is implicit, so must change the variant that you are calling to explicitly pass a comparator. Having to choose a different function is not very discoverable.

We could have a comparator transformer to reverse sort order - that would be 'compositional' and reduce the combination space, but for many developers, it would seem arcane.

One might argue that the By variants are unnecessary too, as they can be achieved by generating a comparator from a key selector and key comparator. In practice any combinator approach is inefficient, since the stacked calls often can't be inlined, and the underlying sort algorithm can't do something more efficient like hold a key in a variable or negate the comparator result. Relying on compareTo in a generic or generalized context can be slow due to the convariance check on the parameter.

I don't think the better static errors make IterableComparableExtension.sorted pay its way.

None of these methods mention stability in their documentation, yet some are implemented in terms of List.sort and others in terms of mergeSort. This is an exhibit supporting my contention that nobody worries about sort stability until it bites them. One fix for the unspecified stability is to make the default List.sort stable dart-lang/sdk#433.

The two definitions with the same name sorted are also problematic.
<int>[1].sorted() wont compile, but <num>[1].sorted() does.
The error is that I am missing a required argument.
This is just baffling. None of the CLI tools (cfe, analyser) tell me why (the other sorted was matched).

I would suggest having only two extension methods, IterableExtension.sorted and IterableExtension.sortedBy.
Both sorted and sortedBy would support optional named arguments compare: and descending: false.
If you try to get a sorted list of non-comparables without passing a comparator, you get a runtime error when instantiating the default, but that is less confusing to me that the <int>[1].sorted() error.

I would still add Comparable.compareInverse.

[lrhn]

We could have a comparator transformer to reverse sort order

We do.

Maybe sorting in the opposite order happens often enough that operations that do the same thing in the inverse order would be reasonable. (And maybe a named boolean is easier than coming up with new names, at least if none of the functions take an optional positional argument yet.)

I'm not too worried about a sorting function doing one extra if at the start to select direction. More worried if we have to do a condition inside the sorting loop, or have to duplicate the sorting loop. I would prefer to not wrap the comparator in (a, b)=> compare(b,a). That's extra overhead on each invocation. I'm sure something can be done efficiently.

This is just baffling.

It's baffling every time T extends Comparable<T> fails to solve an input of int into a T of num. Maybe if we get variance annotations. (Comparison is contravariant.)

But then we come to naming: inverse or descending? (Since we don't use "ascending" anywhere, and don't give a direction to comparators - it's just "before" and "after" in comparison order - it's not a slam-dunk to use "descending". But it does paint a better picture than the completely neutral "inverse".)