Problem
I currently have something like this:
Price price = new Price();
ActualValue actualValue = new ActualValue();
actualValue.setValue(price.getPreviousPrice().getRegion().getValue());
I want to make sure when calling getRegion()
and getValue()
, no NPE is thrown, trying to make it write in one line so I thought about using Optional
Currently what I have is this:
Optional.of(price)
.flatMap(d -> Optional.ofNullable(d.getPreviousPrice())
.flatMap(p -> Optional.ofNullable(p.getRegion())
.flatMap(m -> Optional.ofNullable(m.getValue()))))
.ifPresent(v -> actualValue.setValue(v));
Looks ugly, how can I improve?
Solution
Optional.map produces a cleaner code. It’s similar to flatMap except the function inside doesn’t need to know about Optional. Also passing method references makes things a little bit shorter.
Optional.of(price)
.map(Price::getPreviousPrice)
.map(Price::getRegion)
.map(Region::getValue)
.ifPresent(ActualValue::setValue);
My advice is to make rare use of the Optional-construct. It may make the code look “cleaner” and surly “shorter” but it brings a false sense of safety. BTW less code is no metric to follow.
I would come from the other side and ask questions like:
- Do I violate the law of demeter with “price.getPreviousPrice().getRegion().getValue()”?
- Why should a previous price have no region where it is valid?
You should have a look at your design. These new constructs may have their applications. But most of the time I see them in usage they hide design flaws. Semantical problems where obfuscated by language mechanisms.