Abusing Optional#map considering absent Optional#peek method

Question

Consider following classes:

ClassA {
}

ClassB {
    private ClassA classA;

    // getters and setters
}

As there is no peek method available on Optional, is this considered abusing the Optional#map method:

return myService.findA()
        .flatMap(a -> myService.findB(a) // also returns optional
                    .map(b -> {
                            b.setClassA(a);
                            return b;
                        }))
        .map(d -> MyType.of(d)) // MyType being other class
        .orElse(MyType.empty());

This could also be written like this:

Optional<ClassA> optionalA = myService.findA();

if (optionalA.isPresent()) {
    Optional<ClassB> optionalB = myService.findB(optionalA.get());

    if (optionalB.isPresent()) {
        ClassB classB = optionalB.get();
        classB.setClassA(optionalA.get());
        return MyType.of(classB);
}
return MyType.empty();

Both implementations perform the same logic, should one be preferred over the other ?

IMHO, (a) yes, that's abuse, as `map` should probably not have any side effects, and (b) the "old-style" code might be a bot more verbose, but it's much, _much_ clearer and easier to comprehend. — tobias_k, Jul 05 '18 at 15:59
at the same time java doc for peek says: This method exists mainly to support debugging. So it is an open question what to use for state mutation `peek` or `map`. Please see [peek](https://stackoverflow.com/questions/33635717/in-java-streams-is-peek-really-only-for-debugging) and [map vs peek](https://stackoverflow.com/questions/42117419/better-way-than-stream-peek) — Anton Balaniuc, Jul 05 '18 at 16:08
@AntonBalaniuc neither. There is already `ifPresent` in the class `Optional` which is explicitly made for performing an action. — Holger, Jul 05 '18 at 16:24
@tobias_k not "probably" but "absolutely". Methods passed to map should be functions, which are pure by definition. — Rea Sand, Jul 05 '18 at 20:57

score 3 · Answer 1 · answered Jul 05 '18 at 16:22

3

To me, it looks strange that myService.findB(a) doesn’t already imply b.setClassA(a), as it has both in scope, but anyway, there is no need to abuse map here:

return myService.findA()
    .flatMap(a -> {
        Optional<B> oB = myService.findB(a);
        oB.ifPresent(b -> b.setClassA(a));
        return oB;
    })
    .map(MyType::of).orElse(MyType.empty());

answered Jul 05 '18 at 16:22

Holger

285,553
42
434
765

This _is_ much clearer than OP's version (both, IMHO), but isn't this still an abuse of `flatMap` just as it was of `map`? – tobias_k Jul 05 '18 at 22:13
@tobias_k the problem with the `map` in the original version was that it was added merely for performing an action with side-effect; it didn’t really *map* the value to something else. The `flatMap` of my answer still is used in the intended way, to perform an operation that evaluates to another `Optional`. If you’re still not convinced, you can encapsulate `myService.findB(a);` and `oB.ifPresent(b -> b.setClassA(a));` into a new method on its own and call this method in `flatMap`; as said, I’d expect `findB` to do that in the first place when it is part of the operation, semantically. – Holger Jul 06 '18 at 06:01

score 1 · Answer 2 · answered Jul 05 '18 at 21:22

When you apply functional design principles consequently, you don't ever update state in place. This implies to not have any data structures allowing this (as is the case for your ClassB with that setter). You'd rather return a new instance of that class with the updated value(s). And if you do that, there's no need to "abuse" either map or flatMap but you can make use of a closure instead:

return myService.findA()
    .flatMap(a -> myService.findB(a).map(b -> b.copyWith(a)))
    .map(MyType::of)
    .orElse(MyType.empty());

score 0 · Answer 3 · answered Jul 05 '18 at 16:04

0

I'll probably prefer to go with the first implementation since the major feature that Optional offers in transforming, I would assume it would be best to put that to good use.

answered Jul 05 '18 at 16:04

joyidahosa

23
7

The major feature of `Optional` is providing a means to deal with the *effect of absence* in a declarative and convenient way (eventhough `java.util.Optional` is broken) – Rea Sand Jul 05 '18 at 21:26

Abusing Optional#map considering absent Optional#peek method

3 Answers3