How to group a stream to a map by using a specific key and value?

Question

I have a Stream<Pair<String, String>> myStream; and I'd like to aggregate it into a Map<String, Set<String>> result;

I managed to get to the following:

Map<String, Set<Pair<String, String>>> result = myStream
  .collect(Collectors.groupingBy(Pair::getKey, Collectors.toSet()));

This fails with "Non-static method cannot be referenced from a static contex":

Map<String, Set<String>> result = myStream
  .collect(Collectors.groupingBy(Pair::getKey, Pair::getValue, Collectors.toSet()));

What am I doing wrong?

`groupingBy` with 3 arguments is [`groupingBy(Function super T,? extends K> classifier, Supplier mapFactory, Collector super T,A,D> downstream)`](https://docs.oracle.com/javase/8/docs/api/java/util/stream/Collectors.html#groupingBy-java.util.function.Function-java.util.function.Supplier-java.util.stream.Collector-). Second parameter is defined as *"a function which, when called, produces a **new empty Map** of the desired type"*, so why did you think that `Pair::getValue` would be such a factory. — Andreas, Feb 09 '19 at 00:08

score 7 · Answer 1 · answered Feb 09 '19 at 00:14

7

Correct code is:

Map<String, Set<String>> result = myStream
  .collect(Collectors.groupingBy(Pair::getKey,
              Collectors.mapping(Pair::getValue, Collectors.toSet())));

If you use import static, it is shortened to:

Map<String, Set<String>> result = myStream
  .collect(groupingBy(Pair::getKey, mapping(Pair::getValue, toSet())));

answered Feb 09 '19 at 00:14

Andreas

154,647
11
152
247

2

Just as a disclaimer; I try to avoid using `import static`. It makes it less obvious where a particular method is being sourced from. Sometimes I agree with it (for say, heavy use of assert methods or mocking methods in test classes, or heavy use of a constants class), but for one off method calls, I find it cleaner to just use the `Class.method` style of invoking the method. – flakes Feb 09 '19 at 03:10
1

@flakes well, it's three method calls which can be imported with a single `import static java.util.stream.Collectors.*;`. For some sources of static methods, like `java.lang.Math`, `java.util.Arrays`, or this very example, developers should not need a reminder at every single method call. Keep in mind that `import static` has the no precedence at all, it's only used when all other ways of resolving a method, i.e. inheritance or lexical scope, including its inheritance, did not lead to any method of this name. In this regard, imports are not the most obscure way of resolving a method. – Holger Feb 09 '19 at 18:31
@Holger again, this is a disclaimer, not a blanket rule to avoid this style of import. That said, one of my favorite aspects of clean java code is that with minimal context I can look at a class and almost immediately validate where all my components are sourced from. When you introduce static imports (and the more controversial wildcard import) you lose this convenience. As a paranoid code-reviewer this makes my life tougher, as now I have to confirm the validity of these calls. Further, you allow changes to the namespace (through inheritance or package) to invalidate non edited code paths. – flakes Feb 09 '19 at 19:02
@flakes when you see an invocation like `foo()`, it can be a method within the same class containing the invocation, a method inherited by that class, a method of an outer class or a method inherited by the outer class. You would have to scroll up to identify the nested scopes and see their supertypes. These supertypes are usually given by `extends SimpleName` or `implements SimpleName`, so you now have to go up to the imports to identify the actual types. Then you still don't know which of these types (or *their* supertypes) provided the method. What does `import static` make worse here? – Holger Feb 09 '19 at 19:40
@Holger simply put, for the advantage of greater readability they have a cost of increasing the complexity of the symbol resolution. As the scope of the namespace increases, the complexity of resolution grows as well. For concise classes this is usually not a problem, but for longer classes or classes with complex inheritance structures I believe the costs outweigh the gain. It's a paradigm that I do not personally subscribe to. These posts can do better justice than I can in the comments https://stackoverflow.com/a/421127/3280538 https://stackoverflow.com/a/147461/3280538 – flakes Feb 09 '19 at 19:52
@flakes these are very old statements and the second is not even about `import static`. To me, this looks very much like dismissing `import static` for its rather small complexity while accepting the really complex logic of nesting and inheritance, simply because the former is newer. Anyway, whether you write `groupingBy` or `Collectors.groupingBy`, in either case, you are resolving an identifier via an import statement and need to understand it. And in case of `Collectors.groupingBy`, you still have to preclude that `Collectors` is an inherited name, to be sure that it is the imported name... – Holger Feb 09 '19 at 20:07
@Holger The second link was for wildcard imports, which have similar concerns in both regular and static contexts. To your latter point; would you really argue an equal likelyhood of two resolutions existing for `Collectors.groupingBy` vs just `groupingBy`? I'm certainly not suggesting that all class references be fully qualified, however, over-simplifying names can cause problems in the namespace. In fact, I've actually run into production issues for legacy systems that were caused by this. Not fun. IMO this is a case of readability vs code durability. Use `static import` at your own peril. – flakes Feb 09 '19 at 21:28

How to group a stream to a map by using a specific key and value?

1 Answers1