search

google / guava

Google core libraries for Java
Apache License 2.0
50.2k stars 10.91k forks source link

Add Multimaps.index(Set<V>, Function<V, K>) #2468

Open ghost opened 8 years ago

ghost commented 8 years ago

Currently, there's two Multimap.index methods both returning an ImmutableListMultimap. It would be useful if there was a method added alongside with the following signature:

<K, V> ImmutableSetMultimap<K, V> indexSet(Set<V> values, Function<? super V, K> keyFunction);

for the specific situation in which values is a Set, then the returned multimap can be a SetMultimap, as any subset of the values set will also be a valid set. And SetMultimap offers semantic guarantees on value uniqueness that is not guaranteed by Multimap or ListMultimap.

Of course, this method will need a different name to not overload the existing index method to maintain source compatibility.

lowasser commented 8 years ago

Do you have an actual use case where the value collection is a Set? Can you discuss it?

ghost commented 8 years ago

In this example, we've got a method that takes a SetMultimap (that depends on the semantics provided by that class, namely, unique values per key). I want to use the output of Multimaps.index as the parameter to that method, but that always returns a ListMultimap, even if the input is a Set. So I'm forced to do an additional ImmutableSetMultimap.copyOf for no reason.

Coincidentally, the method requiring the SetMultimap is the same as mentioned in my last comment in #2487

cpovirk commented 8 years ago

Calls to ImmutableSetMultimap.copyOf(Multimaps.index(...)) account for about half a percent of all Multimaps.index calls in Google's code. But probably SetMultimap would be a good choice for a decent fraction of all callers. For starters, it would be interesting to see how many calls to index pass a Set.

cpovirk commented 5 years ago

Stream support also chips away at the need for this. Still, we could consider it someday.