Reputation: 111
I would like got your advise on how to properly write this code on a functional way:
private Option<CalcResult> calculate(Integer X, Integer Y) {
if (X < Y) return Option.none();
return Option.of( X + Y );
}
public Option<CalcResult> otherMethod(Obj o) {
if (o.getAttr()) {
// getA() & getB() are APIs out of my control and could return a null value
if (o.getA() != null && o.getB() != null) {
return calculate(o.getA(), o.getB());
}
}
return Option.none();
}
Calculate is simple:
private Option<CalcResult> calculate(Integer X, Integer Y) {
return Option.when(X > Y, () -> X + Y);
}
For otherMethod
, This was my first approach:
public Option<CalcResult> otherMethod(Obj o) {
return Option.when(o.getAttr(), () ->
For(Option.of(o.getA()), Option.of(o.getB()))
.yield(this::calculate)
.toOption()
.flatMap(Function.identity())
).flatMap(Function.identity());
}
But, I feel the code is not as readable as I would expect, compared to first version (double flatMap
makes hard to understand, at first sight, why is there)
I tried with this other, which improves the lecture:
public Option<CalcResult> otherMethod(Obj o) {
return For(
Option.when(o.getAttr(), o::getAttr()),
Option.of(o.getA()),
Option.of(o.getB()))
.yield((__, x, y) -> this.calculate(x, y))
.toOption()
.flatMap(Function.identity());
}
It's way more readable, but I think i'm not correctly using For-comprehension on this case.
What would be your recommendation on this case? am I correctly using vavr's API?
Thanks
Upvotes: 1
Views: 827
Reputation: 1833
I would write calculate
exactly the way you did (except I would never use upper case for parameters :P).
As for otherMethod
, I would use pattern matching. Pattern matching in Vavr is not even close to PM in more functional languages like Haskell, but I think it still correctly represents your intention:
public Option<Integer> otherMethod(Obj o) {
return Option.when(o.getAttr(), () -> Tuple(Option(o.getA()), Option(o.getB()))) //Option<Tuple2<Option<Integer>, Option<Integer>>>
.flatMap(ab -> Match(ab).option( // ab is a Tuple2<Option<Integer>, Option<Integer>>
Case($Tuple2($Some($()), $Some($())), () -> calculate(o.getA(), o.getB())) // Read this as "If the pair (A, B)" has the shape of 2 non-empty options, then calculate with what's inside
// Result of Match().option() is a Option<Option<Integer>>
).flatMap(identity())); // Option<Integer>
}
Alternative, without comments (note we use Match().of()
instead of Match().option()
, so we HAVE to deal with all possible shapes):
public Option<Integer> otherMethod(Obj o) {
return Option.when(o.getAttr(), () -> Tuple(Option(o.getA()), Option(o.getB())))
.flatMap(ab -> Match(ab).of(
Case($Tuple2($Some($()), $Some($())), () -> calculate(o.getA(), o.getB())),
Case($(), () -> None())
));
}
I replaced CalcResult
with Integer
since in your example, calculate
really returns an Option<Integer>
, I let you adapt to your business models.
Upvotes: 1