Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Translate complex conditional logic inside a loop into streams and lambdas

I'm looking for a clean way to translate complex logical conditions with if and else statements that lead to different actions, into lambdas and streams.

Suppose I have this code:

List<OuterData> result = new LinkedList<>();

for (Outer outer : getOutersFromSomewhere()) {
    OuterData outerData = new OuterData();

    if (outer.isImportant()) {
        doImportantAction(outer, outerData);
    } else if (outer.isTrivial()) {
        doTrivialAction(outer, outerData);
    } else {
        doDefaultAction(outer, outerData);
    }

    for (Inner inner : outer.getInners()) {
        if (inner.mustBeIncluded()) {
            InnerData innerData = new InnerData();

            if (inner.meetsCondition1()) {
                doAction1(inner, innerData, outer, outerData);
            } else if (inner.meetsCondition2()) {
                doAction2(inner, innerData, outer, outerData);
            } else {
                doDefaultAction(inner, innerData, outer, outerData);
            }
            outerData.add(innerData);
        }
    }
    result.add(outerData);
}

return result;

This is simplified from real code I have. I know it can be optimized and refactored, i.e. I could move inner for to a private method. I'd like to know how to translate the if, else if and else parts to streams and lambdas.

I know how to translate the skeleton of this example. I'd use List.stream(), Stream.map(), Stream.filter(), Stream.collect() and Stream.peek(). My problem is with conditional branches only. How can I do this translation?

like image 427
fps Avatar asked Mar 20 '15 15:03

fps


1 Answers

One first obvious way is to stream your elements, filter them according to the needed criteria, and then applying the action on each remaining element. This also makes the code much cleaner:

List<Outer> outers = getOutersFromSomewhere();
outers.stream().filter(Outer::isImportant)
    .forEach(outer -> doImportantAction(outer, outerDate));
outers.stream().filter(Outer::isTrivial)
    .forEach(outer -> doTrivialAction(outer, outerDate));
// default action analog

Caution: This only works if the important, the trivial, and the default elements form a partition. Otherwise it is not equivalent to your if-else-structure. But maybe this is intended anyway ...

The main problem with this approach: It is not very good OOP. You are querying the objects in order to make a decision. But OOP should be "tell, don't ask" as much as possible.

So another solution is to provide a consuming method in your Outer class:

public class Outer {
    ...
    public void act(OuterData data, Consumer<Outer> importantAction,
            Consumer<Outer> trivialAction, Consumer<Outer> defaultAction) {
        if (isImportant())
            importantAction.accept(this, data);
        else if (isTrivial())
            trivialAction.accept(this, data);
        else
            defaultAction.accept(this, data);
    }
}

Now you call it as simple as this:

List<Outer> outers = getOutersFromSomewhere();
outers.forEach(outer -> outer.act(...)); // place consumers here (lambdas)

This has a clear advantage: If you ever have to add a feature to your Outer class - let's say isComplex() - you have to only change the internals of that single class (and maybe resolve the compiler failure in other parts). Or maye you can add this feature in a backward compatible way.

The same rules can be applied to the Inner class and the iteration.

like image 184
Seelenvirtuose Avatar answered Sep 25 '22 13:09

Seelenvirtuose