15

Let's say I have some stream and want to collect to map like this

stream.collect(Collectors.toMap(this::func1, this::func2));

But I want to skip null keys/values. Of course, I can do like this

stream.filter(t -> func1(t) != null)
    .filter(t -> func2(t) != null)
    .collect(Collectors.toMap(this::func1, this::func2));

But is there more beautiful/effective solution?

Joel
  • 473
  • 2
  • 7
  • 22
  • I'm voting to close this question as off-topic because questions about working code belong on [Code Review](https://codereview.stackexchange.com/). – Michael Apr 03 '18 at 15:11
  • Since this is your methods `func1` and `func2`, one of the ways you can beautify this is by having a third method which would act as a predicate filter. – M. Prokhorov Apr 03 '18 at 15:13

5 Answers5

13

If you want to avoid evaluating the functions func1 and func2 twice, you have to store the results. E.g.

stream.map(t -> new AbstractMap.SimpleImmutableEntry<>(func1(t), func2(t))
      .filter(e -> e.getKey()!=null && e.getValue()!=null)
      .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

This doesn’t make the code shorter and even the efficiency depends on the circumstances. This change pays off, if the costs of evaluating the func1 and func2 are high enough to compensate the creation of temporary objects. In principle, the temporary object could get optimized away, but this isn’t guaranteed.

Starting with Java 9, you can replace new AbstractMap.SimpleImmutableEntry<>(…) with Map.entry(…). Since this entry type disallows null right from the start, it would need filtering before constructing the entry:

stream.flatMap(t -> {
   Type1 value1 = func1(t);
   Type2 value2 = func2(t);
   return value1!=null && value2!=null? Stream.of(Map.entry(value1, value2)): null;
 })
 .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

Alternatively, you may use a pair type of one of the libraries you’re already using (the Java API itself doesn’t offer such a type).


Another (probably the most efficient) alternative is a custom collector:

stream.collect(HashMap::new,
    (m, o) -> {
      Type1 key   = func1(o);
      Type2 value = func2(o);
      if(key != null && value != null) m.put(key, value);
    },
    Map::putAll);

Note that this collector, unlike the original toMap collector, doesn’t check for duplicates. But such a check could be added without problems.

Holger
  • 285,553
  • 42
  • 434
  • 765
  • @Michael I'd steer away from any javafx imports tbh – FrederikVH Apr 03 '18 at 15:26
  • @FrederikVH It's fine for the time being, though it'll be decoupled from the JDK in JDK 11. – Michael Apr 03 '18 at 15:27
  • @Michael Yeah that's what I was thinking, we're on the same page :) – FrederikVH Apr 03 '18 at 15:28
  • 2
    @Michael starting with Java 9, you can use the much simpler `Map.entry(…)`. I’ve added a note to the answer. It’s not only shorter in source code, the result being [value-based](https://docs.oracle.com/javase/9/docs/api/java/lang/doc-files/ValueBased.html) makes it even more suitable for temporary objects. – Holger Apr 03 '18 at 15:28
  • Good shout. `Map.entry` is the best of both worlds. – Michael Apr 03 '18 at 15:38
  • @Holger in your second example, should you not filter null values before collecting to prevent `NullPointerException`: `.filter(Objects::nonNull).collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));` ? – davidxxx Apr 03 '18 at 15:57
  • 3
    @davidxxx not necessary in the case of [`flatMap`](https://docs.oracle.com/javase/8/docs/api/java/util/stream/Stream.html#flatMap-java.util.function.Function-) which maps to a stream, not an element: “*If a mapped stream is `null` an empty stream is used, instead.*” – Holger Apr 03 '18 at 16:00
  • @Holger I would have looked in the documentation before :) Thanks for the reference. – davidxxx Apr 03 '18 at 16:02
5

Another way to avoid evaluating the functions twice. Use a pair class of your choice. Not as concise as Holger's but it's a little less dense which can be easier to read.

stream.map(A::doFuncs)
    .flatMap(Optional::stream)
    .collect(Collectors.toMap(Pair::getKey, Pair::getValue));

private static Optional<Pair<Bar, Baz>> doFuncs(Foo foo)
{
    final Bar bar = func1(foo);
    final Baz baz = func2(foo);
    if (bar == null || baz == null) return Optional.empty();
    return Optional.of(new Pair<>(bar, baz));
}

(Choose proper names - I didn't know what types you were using)

Michael
  • 41,989
  • 11
  • 82
  • 128
  • 2
    You could also implement `doFuncs` as `return Optional.ofNullable(func1(foo)) .flatMap(v1 -> Optional.ofNullable(func2(foo)).map(v2 -> new Pair<>(v1, v2)));`; this would also harmonize with `Map.entry`, as both values are already guaranteed to be non-`null` when the pair/entry is constructed. – Holger Apr 03 '18 at 15:47
3

One option is to do as in the other answers, i.e. use a Pair type, or an implementation of Map.Entry. Another approach used in functional programming would be to memoize the functions. According to Wikipedia:

memoization or memoisation is an optimization technique used primarily to speed up computer programs by storing the results of expensive function calls and returning the cached result when the same inputs occur again.

So you could do it by caching the results of the functions in maps:

public static <K, V> Function<K, V> memoize(Function<K, V> f) {
    Map<K, V> map = new HashMap<>();
    return k -> map.computeIfAbsent(k, f);
}

Then, use the memoized functions in the stream:

Function<E, K> memoizedFunc1 = memoize(this::func1);
Function<E, V> memoizedFunc2 = memoize(this::func2);

stream.filter(t -> memoizedFunc1.apply(t) != null)
    .filter(t -> memoizedFunc2.apply(t) != null)
    .collect(Collectors.toMap(memoizedFunc1, memoizedFunc2));

Here E stands for the type of the elements of the stream, K stands for the type returned by func1 (which is the type of the keys of the map) and V stands for the type returned by func2 (which is the type of the values of the map).

fps
  • 33,623
  • 8
  • 55
  • 110
1

This is a naive solution, but does not call functions twice and does not create extra objects:

List<Integer> ints = Arrays.asList(1, null, 2, null, 3);
Map<Integer, Integer> res = ints.stream().collect(LinkedHashMap::new, (lhm, i) -> {
    final Integer integer1 = func1(i);
    final Integer integer2 = func2(i);
    if(integer1 !=  null && integer2 != null) {
        lhm.put(integer1, integer2);
    }
}, (lhm1, lhm2) -> {});
gil.fernandes
  • 12,978
  • 5
  • 63
  • 76
0

You could create a isFunc1AndFunc2NotNull() method in the current class :

boolean isFunc1AndFunc2NotNull(Foo foo){
   return func1(foo) != null && func2(foo) != null;
}

And change your stream as :

stream.filter(this::isFunc1AndFunc2NotNull)
      .collect(Collectors.toMap(this::func1, this::func2));
davidxxx
  • 125,838
  • 23
  • 214
  • 215