Skip to content

Added Iterable<Comparable> Extensions: .min and .max #42000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

ChristianKleineidam
Copy link

As discussed in #41902 I created two extensions to Iterable. .min that returns the smallest element and .max return s the largest one.

When it comes to the question of what to do when multiple elements have the same value I orientied myself with Pythons implementation of min() and max() that returns that both return the first value.

I copied the headers from other files in their respective folders.

…s the smallest element in the iterable and .max returns the largest one
@parlough
Copy link
Member

For reference the CL for this is here: https://dart-review.googlesource.com/c/sdk/+/148744

@lrhn
Copy link
Member

lrhn commented May 26, 2020

This is not a bad idea. I'd like to consider implications a little more before landing our first extension methods in the platform libraries.

For example, should it be a method instead of a getter? Then we can do:

extension IterableExtension<T> on Iterable<T> {
  T max(int compare(T a, T b)) { 
    var it = this.iterator;
    if (!it.moveNext()) throw StateError("no element");
    var max = it.current;
    while (it.moveNext()) {
      var next = it.current;
      if (max.compareTo(next) < 0) max = next;
    }
    return max;
  }
}
extension IterableComparableExtension<T extends Comparable<T>> on Iterable<T> {
  T max([int compare(T a, T b)]) => 
      IterableExtension(this).max(compare ?? (a, b) => a. compareTo(b));
}

That would be a more general API, and would still allow you to get the maximum of comparables without providing a comparator.

(I think we might indeed want that, and I'm currently working on something similar aimed for package:collection).

@ChristianKleineidam
Copy link
Author

To me it appears as if the max() you define on the general Iterable doublicates the functionality of the existing .reduce()-method and as such provides no additional value.

@lrhn
Copy link
Member

lrhn commented May 26, 2020

It's not exactly the same as reduce.

If you have

T Function(T, T) maxBy<T>(int compare(T a, T b)) => (T a, T b) => compare(a, b) < 0 ? b : a;

then you can indeed do values.max(compare) as values.reduce(maxBy(compare)). It's still more convenient to have it directly, which is why this is a extension method rather than a method on Iterable itself.

I have no problem having more than one way to do a thing, as long as the more specialized one is often useful and more readable.

@ChristianKleineidam
Copy link
Author

You are right that gives additional value. Given that sort() also takes such a compare function, it would also reuse existing patterns to do it that way.

Having a function would also mean that you can pass a default value for cases where the Iterator is empty like Pythons max and min functions have.

@PiN73
Copy link

PiN73 commented May 29, 2020

Using method instead of getter might make sense because it's O(N)

@mit-mit
Copy link
Member

mit-mit commented Nov 2, 2020

Closing as assumed stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants