Guava: ImmutableEnum{Set,Map} Collectors

[michaelhixson]

Add collectors for enum-specialized ImmutableSet and ImmutableMap:

• toImmutableEnumSet()
• toImmutableEnumMap(keyFunction, valueFunction)
• toImmutableEnumMap(keyFunction, valueFunction, mergeFunction)

Providing these collectors in Guava would be a matter of convenience. Users could implement functionally-equivalent collectors themselves, but I think most users would not do so. I think enum-specialized collections are underused and that it is worthwhile to promote their usage and remove friction / barriers to entry.

For example, if a user has code like this:

ImmutableSet<Month> months = stream.collect(ImmutableSet.toImmutableSet());

... then it would be cool if they could optimize their code like this:

ImmutableSet<Month> months = stream.collect(ImmutableSet.toImmutableEnumSet());

... and it would be lame if they had to do this (it would be so lame that they wouldn't bother):

ImmutableSet<Month> months = stream.collect(SomeHelper.toImmutableEnumSet(Month.class));

// Elsewhere... javadoc and tests omitted...
public static <E extends Enum<E>> Collector<E, ?, ImmutableSet<E>>
toImmutableEnumSet(Class<E> elementType) {
  Objects.requireNonNull(elementType);
  return Collectors.collectingAndThen(
      Collectors.toCollection(() -> EnumSet.noneOf(elementType)),
      Sets::immutableEnumSet);
}

[tli2]

Hi: I have been working on this issue and discovered that the fauxverride tests are failing because the original equals code no longer works. Specifically, K extends Enum<K> as a type literal's equals method depends on the place where it is declared and its name. (The code can be found under sun.reflect.generics.reflectiveObjects.TypeVariableImpl's equals method).

I was able to fix the tests by simply comparing the result of Type::getTypeName, however, this is not correct as it is also sensible to the specific name of the generics declared.

Maybe this should be its own issue? I am not sure what the correct fix would be. Any input would be much appreciated!

[michaelhixson]

@tli2 Oh, gross! I hadn't considered that ImmutableSortedSet and friends might "inherit" these methods.

Part of me wants to call that a problem with FauxveridesTest. I don't understand the reason that MethodSignature cares about the generic type parameters for equality. As long as the method names and parameter types are the same, shouldn't javac enforce that the generic type parameters are also the same? (I'm sure I'm missing something.)

In any case, I don't think it's essential that these methods live in ImmutableSet and ImmutableMap. That home would be good for discoverability, but maybe that home would be bad for other reasons that include this fauxverides issue. If these methods instead lived next to Sets.immutableEnumSet(...) and Maps.immutableEnumMap(...) (for example), they would still be useful.

[tli2]

Fair enough. I can move my code. Do you think I should report a separate issue on the test?

[michaelhixson]

Fair enough. I can move my code. Do you think I should report a separate issue on the test?

Unless it's causing other problems, I wouldn't bother. I'm just a random guy from the internet though...

[lowasser]

I've put this into Guava's API review queue. I've put together a different implementation, though, that avoids quite so much redundant copying. I'll keep this thread updated on the status of the API review.

[Stephan202]

Guava 21.0 shipped with com.google.common.collect.Sets.toImmutableEnumSet. If the Guava team does not plan to add the other two types of collector, then this issue can be closed.

[michaelhixson]

They did add the other two as well. Thanks!

[Stephan202]

D'oh I was looking in the wrong place. You're right! Nice.