dart-lang / collection

The collection package for Dart contains a number of separate libraries with utility functions and classes that makes working with collections easier.
https://pub.dev/packages/collection
BSD 3-Clause "New" or "Revised" License
372 stars 86 forks source link

Parameter of `sorted` differs from `sort` #321

Closed vincent-hoodoo closed 9 months ago

vincent-hoodoo commented 10 months ago

Hello,

IterableExtension's sorted has a required positional parameter:

List<T> sorted(Comparator<T> compare)

However, its native counterpart sort has an optional positional parameter:

void sort([int Function(T, T)? compare])

Is there a reason why the parameters are different? If no, could we make sorted's parameter optional?

I'd be happy to open a PR if needed.

lrhn commented 9 months ago

Yes, there is a reason.

The List.sort method API is unsafe. If you call it without a comparator argument on a list of elements that do not implement Comparable, it will just throw at runtime.

The extension approach used here is superior in that you cannot omit the compare function when it's needed, but you can when it is a list of Comparables, because of a more specific extension on List<Comparable>s.

So we were doing the List API again today, it would probably look more like this.

vincent-hoodoo commented 9 months ago

Thank you for the detailed reply @lrhn !

but you can when it is a list of Comparables, because of a more specific extension on Lists.

I've searched for sorted in the repo but couldn't find the more specific extension you mention: https://github.com/search?q=repo%3Adart-lang%2Fcollection+sorted&type=code.

Did you mean this extension? https://github.com/dart-lang/collection/blob/2d57a82ad079fe2d127f5a9b188170de2f5cdedc/lib/src/list_extensions.dart#L292

It includes three functions, but not sorted. Am i missing something?

When I call .sorted() on a List<Comparable> (in my case, a List<double>), the analyzer complains that it's missing a comparator argument.

lrhn commented 9 months ago

The extension I was thinking of is https://github.com/dart-lang/collection/blob/master/lib/src/iterable_extensions.dart#L901

That doesn't work on int/double because they are Comparable<num>. We're adding specific extensions on int and double to get around that (At least until we get variance annotations, Comparable is contravariant, which is why extension doesn't work.)