Skip to content

Add Collection#sorted, Collection#min, Collection#max #1319

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
DartBot opened this issue Jan 24, 2012 · 13 comments
Closed

Add Collection#sorted, Collection#min, Collection#max #1319

DartBot opened this issue Jan 24, 2012 · 13 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-as-intended Closed as the reported issue is expected behavior type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Jan 24, 2012

This issue was originally filed by @seaneagan


List#sort is useful for sorting a list in place, but often you don't want to mutate the list, but rather get a sorted copy of it, and also any collection can be sorted, not just lists, but right now you have to do something like:

List sortedList = new List.from(collection);
sortedList.sort(comparator);

which is inconvenient, but more importantly means that all the elements must be sorted (slow) even to do basic calculations on the returned list, such as "list.last()". Thus, it would be nice to have a separate sorting method in Collection which returns a sorted "view" (see issue #1235) of the Collection:

// comparator optional (see issue #1305)
List<E> sorted([int compare(E a, E b)]);

The two most common use cases for sorting are to calculate a "min" or "max", so those would be nice to have in Collection as well:

E min([int compare(E a, E b)]); // => sorted(compare)[0]
E max([int compare(E a, E b)]) // => sorted(compare).last()

@dgrove
Copy link
Contributor

dgrove commented Jan 25, 2012

Removed Type-Defect label.
Added Type-Enhancement, Area-Library, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Feb 13, 2012

This comment was originally written by @seaneagan


* If no self-mutating List#sort is needed, could rename Collection#sorted to Collection#sort

  • the "contract" for "min" and "max" could be something like:

E min<T>([int compare(E a, E b)]) {
  if(compare == null) compare = (a, b) => a.compareTo(b);
  min(a, b) => compare(a, b) <= 0 ? a : b;
  return reduce(min); // Collection#reduce
}
E max<T>([int compare(E a, E b)]) {
  if(compare == null) compare = (a, b) => a.compareTo(b);
  max(a, b) => compare(a, b) >= 0 ? a : b;
  return reduce(max);
}

basically assume transitivity of the comparator(s) (in order to get O(n) performance), and when two items compare equal, keep the earliest iterated one.

@rakudrama
Copy link
Member

A key function is often more convenient that a comparator.
These functions should take both a key and a comparator.

I like Python's min/max functions.
The min returns the element with the lowest key which we expect to be comparable.

var strs = ['short', 'medium', 'longest'];
max(strs); // --> 'short' - lexicographic compare
max(strs, key: (s) => s.length); // --> 'longest'

@DartBot
Copy link
Author

DartBot commented Nov 29, 2012

This comment was originally written by @seaneagan


I'm thinking just have them take one dynamic argument, which can either be a Comparator or a key function which might be defined as:

typedef num Metric<E>(E e);

Then if/when we get union types, this single argument can be typed as a Comparator | Metric. Along with the latest library changes of moving everything to Iterable and including Stream, this could look like:

class Iterable<E> {
  //...
  E min([var sorter]);
  E max([var sorter]);
  List<E> sorted([var sorter]);
}

class Stream<E> {
  //...
  Future<E> min([var sorter]);
  Future<E> max([var sorter]);
  Future<List<E>> sorted([var sorter]);
}

@lrhn
Copy link
Member

lrhn commented Dec 18, 2012

Issue #7430 has been merged into this issue.

@DartBot
Copy link
Author

DartBot commented Jan 2, 2013

This comment was originally written by @seaneagan


Looks like min and max were added to lib_v2 in https://codereview.chromium.org/11727007/. Thanks!

Is lazy sorting of Iterables and Streams, i.e. the "sorted" method above, also planned? do we need a separate issue for it?

@lrhn
Copy link
Member

lrhn commented Aug 22, 2013

Lazy sorting is not planned. The few operations that can be done without sorting (length, first -> min, last -> max) are not really worth the complexity. Also, it will be a dangerous "view", because you will need to sort for most operations, and any change to the underlying list will mean you need to sort again.

With cascades, it's not bad to do: (list.toList()..sort(comparator)) to get a new sorted list.

The min and max methods were removed again from Iterable.

Maybe we should add min/max functions to comparable:
  static Comparable min(Comparable a, Comparable b)
    => a.compareTo(b) <= 0 ? a : b;
  static Comparable max(Comparable a, Comparable b)
    => a.compareTo(b) >= 0 ? a : b;

Then we can find the minimum of a list of comparables as:
  list.reduce(Comparable.min);

@alan-knight
Copy link
Contributor

(list.toList()..sort(comparator)) isn't too bad in terms of code length, but it's not going to be an obvious choice to anyone new to the language.

@DartBot
Copy link
Author

DartBot commented Aug 22, 2013

This comment was originally written by @seaneagan


Static methods would definitely be better than nothing, but you lose the
return type then (Comparable instead of int for example). See issue #3176.

@peter-ahe-google
Copy link
Contributor

Would a method named toSortedList([compare]) make sense?

@floitschG
Copy link
Contributor

toSortedList() would work, but imho it's not worth it. It doesn't really buy much compared to toList()..sort().

Yes toList()..sort() is not completely obvious but the next best thing is:
var list = x.toList();
list.sort();

while this is slightly longer it isn't that bad and is readable.
I like the fact that we make it clear that sorting requires to cache all elements and therefore having it in a list is just natural.


Added AsDesigned label.

@DartBot
Copy link
Author

DartBot commented Dec 15, 2013

This comment was originally written by @mezoni


Lazy sorting. Result is IQueryable.
Custom comparator can be passed as an argument.

import 'package:queries/collections.dart';

void main() {
  var fruits = ["grape", "passionfruit", "banana", "mango", "orange", "raspberry", "apple", "blueberry"];
  
  var sorted = new Collection(fruits)
    .orderBy((fruit) => fruit.length)
    .thenBy((fruit) => fruit);
  
  for(var fruit in sorted) {
    print(fruit);
  }
}

Output:
mango
banana
orange
blueberry
raspberry
passionfruit

Sorting algorithm is stable (Symmerge sort).

@DartBot DartBot added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-as-intended Closed as the reported issue is expected behavior labels Dec 15, 2013
@mezoni
Copy link

mezoni commented Jun 9, 2015 via email

@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed type-enhancement labels Mar 1, 2016
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-as-intended Closed as the reported issue is expected behavior type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

9 participants