google / guava

Google core libraries for Java
Apache License 2.0
49.91k stars 10.83k forks source link

Implicit natural ordering for collectors for `ImmutableSorted{Set,Map}` #2733

Open scranen opened 7 years ago

scranen commented 7 years ago

Let's say I want to convert a list list to an immutable sorted set set:

List<Integer> list; // Source
ImmutableSortedSet<Integer> set; // Target

When creating a sorted, immutable copy of an iterable or iterator over Comparables, I have the option to leave the ordering implicit:

set = ImmutableSortedSet.copyOf(list);

For the streaming API, I would love to see the same option:

// Currently needed:
set = list.stream().collect(toImmutableSortedSet(naturalOrder()));
// What I want to use:
set = list.stream().collect(toImmutableSortedSet());
lowasser commented 7 years ago

We've provided these for our sorted collections in the past, but I'm not sure we're convinced yet that we should add these overloads to the Collectors. Some Java 8 APIs, such as Stream.sorted, require a Comparator and don't provide another overload -- and the statically imported naturalOrder() in your current code reads pretty well as-is. We'll think about this, though.

dinhani commented 7 years ago

I also would like to have the SortedSet with natural ordering as default when no parameter is passed. I had to create a custom collector to have this functionality.

public static <E extends Comparable<E>> Collector<E, ?, ImmutableSortedSet<E>> toImmutableSortedSet() {
        return ImmutableSortedSet.toImmutableSortedSet(Ordering.natural());
}
cpovirk commented 4 months ago

One way in which toImmutableSortedSet differs from Stream.sorted is that toImmutableSortedSet, as a static method, could restrict itself to being used only with Comparable types.

Wait, also: A no-arg Stream.sorted() seems to have existed from the beginning, despite the ClassCastException danger :) But it remains true that the JDK has some precedent for requiring a Comparator, like in Collectors.maxBy (which is static and thus like toImmutableSortedSet) and Stream.max (non-static, like Stream.sorted). (This discussion sounds vaguely familiar to some past JDK feature request or thread, but I haven't dug up any references....)

cpovirk commented 4 months ago

(It looks like a hair ~under~ over 70% of the callers in Google's codebase pass Comparator.naturalOrder() / Ordering.natural() / Something::compareTo. A few others pass Something::compare in cases in which that's equivalent. And I see a few more who stuff Ordering.natural() into a constant and then use that.)

cpovirk commented 4 months ago

One interesting question would be how many users pass a Comparable type but a non-natural-order Comparator.

I can at least pull some examples quickly: I see ~6% of callers who pass String.CASE_INSENSITIVE_ORDER and ~1% who pass Comparator.reverseOrder() or an equivalent.