Allow min and max to work with Comparable's like DateTime

[ChristianKleineidam]

I have a list of DateTime string and want to know the latest DateTime in my list. If DateTime would not only provide .isBefore() and .isAfter() I should be able to get the latest DateTime in my list with max (from dart:math).

While I do grant that .isBefore() and .isAfter() are normally more readable, they aren't as easy to use with existing functionality like max and him so it would be useful to have the syntax for both.

[mraleph]

Note that min and max only work with numbers (their type parameter has num as a bound) and not with arbitrary comparable objects. So overriding < on DateTime would not permit you to use it with min/max.

There is no convenient method in the core library, but kt_dart package has min and max extension methods on Iterable<T extends Comparable<T>>, so if you have a List<DateTime> l you should be able to just do l.min() and l.max(). See https://pub.dev/documentation/kt_dart/latest/kt_collection/KtComparableIterableExtension.html

[ChristianKleineidam]

It seems > and < aren't implemented because compareTo() and == have different behavior. == not only cares about the time the DateTime happens but also about the timezone that's set.

Looking a bit more at the issue it seems the problem is the max working on num and not on comparable.

Is there a good reason for min and max to not work for comparable?

[mraleph]

/cc @lrhn for library change request.

[lrhn]

I'd prefer to not make min and max from dart:math work on other types than numbers.
One of the reasons is that NaN behaves differently when doubles are used as Comparable, and I'd prefer if max(NaN, 1) keeps being NaN, not 1.
We could special-case numbers, but that would add extra overhead.

I'd rather add static min and max methods on Comparable:

class Comparable<T> {
  static V min<V extends Comparable<V>>(V first, V second) => 
    first.compareTo(second) <= 0 ? first : second;
  static V max<V extends Comparable<V>>(V first, V second) => 
    first.compareTo(second) >= 0 ? first : second;
  ...

Maybe even V minAll<V extends Comparable<V>>(Iterable<V> values) => ....

[ChristianKleineidam]

Okay, then it makes sense to have the functionality for min/max with Comparables outside of the math package.

How about a getter on Iterable<Comparable>? Then I could call [Duration(hours: 1),Duration(hours: 2)].max which is very concise and also easy to discover?

[lrhn]

That is something I'm considering adding, potentially as part of package:collection.

[ChristianKleineidam]

I did try to define this as an extension:

extension MaxCompare on Iterable<Comparable> {
  Comparable get max {
    return this.reduce((Comparable value, Comparable element) =>
        value.compareTo(element) > 0 ? value : element);
  }
}

For some reason I don't understand the following test fails:

 test('Test sum', () {
    expect([2, 1].max, 2);
  });

With the error type '(Comparable<dynamic>, Comparable<dynamic>) => Comparable<dynamic>' is not a subtype of type '(int, int) => int' of 'combine'
Did I make a mistake in the definition of is this a dart bug?

[rakudrama]

It was my thought too that we should be able to use an extension:

extension MaxCompare<T extends Comparable<T>> on Iterable<T> {
  T get max {
    return reduce((a, b) => a.compareTo(b) >= 0 ? a : b);
  }
}

main() {
  print([Duration(minutes:1), Duration(hours: 2), Duration(seconds: 3)].max); // 2:00:00.000000
}

@lrhn what is unfortunate about this is that it does not work for List<double> for List<int>:

Error: The getter 'max' isn't defined for the class 'List<int>'.
 - 'List' is from 'dart:core'.
Try correcting the name to the name of an existing getter, or defining a getter or field named 'max'.
  print([1,2].max);
              ^^^

[lrhn]

Yeah, it's annoying that int implements Comparable<num> which means it's not Comparable<int>. (And it is Comparable<Object> according to the type system, even though it definitely isn't).

If/when we get variance support, the Comparable type argument should become contravariant.

[mraleph]

I thought you could do

extension MaxCompare<U extends T, T extends Comparable<T>> on Iterable<U> {
  // ...
}

but apparently that does not really do anything, which is a pity, so essentially you can't do this in a safe manner and forced to do

extension MaxCompare<T extends Comparable> on Iterable<T> {
  // ...
}

[i-jk]

At the moment if you try to min(date1, date2) - or max - you get a complaint that one of the dates is null, when it really isn't.

Dart should instead throw an accurate and much more appropriate exception as this behaviour is rather confusing.

[cbatson]

See also #3176 "min, max, and clamp apply to all Comparables"

[natebosch]

We have IterableComparableExtension in package:collection which has these API.

https://pub.dev/documentation/collection/latest/collection/IterableComparableExtension.html

I don't think we plan to move the functionality into the SDK, but we can reopen this issue if those plans change.