Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is there an elegant way to convert a Map<P, Optional<Q>> to a sparse Map<P, Q>?

Tags:

Is there an elegant way to convert a Map<P, Optional<Q>> to a sparse Map<P, Q> ?

This should work, but it's a bit meh:

Map<P,Optional<Q>> map = ...;
Map<P,Q> map2 = map.entrySet()
                  .stream().filter(e -> e.getValue().isPresent())
                  .collect(Collectors.toMap(e -> e.getKey(), e->e.getValue().get()));
like image 280
Paul Janssens Avatar asked Feb 18 '19 12:02

Paul Janssens


1 Answers

I would say your way almost already is the most elegant way, I'd only do some slight cosmetic changes and replace e -> e.getKey() in your collector with Entry::getKey. This is only a small change, but communicates your intent better then the other lambda.

Map<P, Optional<Q>> map = new HashMap<>();
Map<P, Q> sparseMap = map.entrySet().stream()
    .filter(e -> e.getValue().isPresent())
    .collect(Collectors.toMap(Entry::getKey, e -> e.getValue().get()));

Why aren't the other solutions better / more elegant?

Because they aren't more concise, and they again fall into the trap of not declaring what you want to do, but how, which is common in procedural styles, but not so much in functional ones.

If you look at the above code, it is pretty much self-explanatory and has a nice flow to it. You first have a non-sparse map with Optionals, then declare the sparse map without Optionals and then describe the transformation of the former map into the latter. This has also no side-effects. The sparse map is assigned only when the collector is actually finished.

If you look at other solutions, those invert the logic flow and use the procedural way of thinking:

Map<P, Optional<Q>> map = [....];
Map<P, Q> sparseMap = new HashMap<>();
map.forEach((key, opt) -> opt.ifPresent(value -> sparseMap.put(key, value)));

This is only marginally shorter then:

Map<P, Optional<Q>> map = [....];
Map<P, Q> sparseMap = new HashMap<>();
for (Entry<P, Optional<Q>> e : map.entrySet()) e.getValue().ifPresent(value -> sparseMap.put(key, value))

You save a few chars due to type inference, but in the end, if you format them reasonably, both foreach solutions need 4 LOC, so they aren't shorter then the functional one. They aren't clearer, either. On the contrary, they rely on causing side-effects in the other map. Which means during computation, you get a partially constructed sparse map assigned to your variable. With the functional solution, the map is only assigned when it is properly constructed. This is only a small nitpick, and likely not going to cause issues in this case, but is something to keep in mind for other situations where it might become relevant (e.g. when concurrency is involved), especially when the other map isn't a local variable but a field -- or worse, passed in from somewhere else.

Furthermore, the functional approach scales better - switching to a parallel stream if you have lots of data is trivial, converting the foreach-approach to parallel requires rewriting to the functional filter/collect approach anyways. This isn't relevant for such lightweight operations (in fact, do not do it here, its likely slower), but in other situations that might be a desirable characteristic.

In my opinion, using the functional filter/collect approach is preferable to using the procedural foreach, because you train yourself to use good habits. But keep in mind that "elegance" is often in the eye of the beholder. To me, the more "elegant" way is the proper functional way which has no side effects. YMMV.

like image 160
StandByUkraine Avatar answered Sep 30 '22 14:09

StandByUkraine