Use Java lambda instead of 'if else'

108,250

Solution 1

As it almost but not really matches Optional, maybe you might reconsider the logic:

Java 8 has a limited expressiveness:

Optional<Elem> element = ...
element.ifPresent(el -> System.out.println("Present " + el);
System.out.println(element.orElse(DEFAULT_ELEM));

Here the map might restrict the view on the element:

element.map(el -> el.mySpecialView()).ifPresent(System.out::println);

Java 9:

element.ifPresentOrElse(el -> System.out.println("Present " + el,
                        () -> System.out.println("Not present"));

In general the two branches are asymmetric.

Solution 2

It's called a 'fluent interface'. Simply change the return type and return this; to allow you to chain the methods:

public MyClass ifExist(Consumer<Element> consumer) {
    if (exist()) {
        consumer.accept(this);
    }
    return this;
}

public MyClass ifNotExist(Consumer<Element> consumer) {
    if (!exist()) {
        consumer.accept(this);
    }
    return this;
}

You could get a bit fancier and return an intermediate type:

interface Else<T>
{
    public void otherwise(Consumer<T> consumer); // 'else' is a keyword
}

class DefaultElse<T> implements Else<T>
{
    private final T item;

    DefaultElse(final T item) { this.item = item; }

    public void otherwise(Consumer<T> consumer)
    {
        consumer.accept(item);
    }
}

class NoopElse<T> implements Else<T>
{
    public void otherwise(Consumer<T> consumer) { }
}

public Else<MyClass> ifExist(Consumer<Element> consumer) {
    if (exist()) {
        consumer.accept(this);
        return new NoopElse<>();
    }
    return new DefaultElse<>(this);
}

Sample usage:

element.ifExist(el -> {
    //do something
})
.otherwise(el -> {
    //do something else
});

Solution 3

You can use a single method that takes two consumers:

public void ifExistOrElse(Consumer<Element> ifExist, Consumer<Element> orElse) {
    if (exist()) {
        ifExist.accept(this);
    } else {
        orElse.accept(this);
    }
}

Then call it with:

element.ifExistOrElse(
  el -> {
    // Do something
  },
  el -> {
    // Do something else
  });

Solution 4

The problem

(1) You seem to mix up different aspects - control flow and domain logic.

element.ifExist(() -> { ... }).otherElementMethod();
          ^                      ^
        control flow method     business logic method

(2) It is unclear how methods after a control flow method (like ifExist, ifNotExist) should behave. Should they be always executed or be called only under the condition (similar to ifExist)?

(3) The name ifExist implies a terminal operation, so there is nothing to return - void. A good example is void ifPresent(Consumer) from Optional.

The solution

I would write a fully separated class that would be independent of any concrete class and any specific condition.

The interface is simple, and consists of two contextless control flow methods - ifTrue and ifFalse.

There can be a few ways to create a Condition object. I wrote a static factory method for your instance (e.g. element) and condition (e.g. Element::exist).

public class Condition<E> {

    private final Predicate<E> condition;
    private final E operand;

    private Boolean result;

    private Condition(E operand, Predicate<E> condition) {
        this.condition = condition;
        this.operand = operand;
    }

    public static <E> Condition<E> of(E element, Predicate<E> condition) {
        return new Condition<>(element, condition);
    }

    public Condition<E> ifTrue(Consumer<E> consumer) {
        if (result == null)
            result = condition.test(operand);

        if (result)
            consumer.accept(operand);

        return this;
    }

    public Condition<E> ifFalse(Consumer<E> consumer) {
        if (result == null)
            result = condition.test(operand);

        if (!result)
            consumer.accept(operand);

        return this;
    }

    public E getOperand() {
        return operand;
    }

}

Moreover, we can integrate Condition into Element:

class Element {

    ...

    public Condition<Element> formCondition(Predicate<Element> condition) {
        return Condition.of(this, condition);
    }

}

The pattern I am promoting is:

  • work with an Element;
  • obtain a Condition;
  • control the flow by the Condition;
  • switch back to the Element;
  • continue working with the Element.

The result

Obtaining a Condition by Condition.of:

Element element = new Element();

Condition.of(element, Element::exist)
             .ifTrue(e -> { ... })
             .ifFalse(e -> { ... })
         .getOperand()
             .otherElementMethod();

Obtaining a Condition by Element#formCondition:

Element element = new Element();

element.formCondition(Element::exist)
           .ifTrue(e -> { ... })
           .ifFalse(e -> { ... })
       .getOperand()
           .otherElementMethod();

Update 1:

For other test methods, the idea remains the same.

Element element = new Element();

element.formCondition(Element::isVisible);
element.formCondition(Element::isEmpty);
element.formCondition(e -> e.hasAttribute(ATTRIBUTE));

Update 2:

It is a good reason to rethink the code design. Neither of 2 snippets is great.

Imagine you need actionC within e0.exist(). How would the method reference Element::actionA be changed?

It would be turned back into a lambda:

e0.ifExist(e -> { e.actionA(); e.actionC(); });

unless you wrap actionA and actionC in a single method (which sounds awful):

e0.ifExist(Element::actionAAndC);

The lambda now is even less 'readable' then the if was.

e0.ifExist(e -> {
    e0.actionA();
    e0.actionC();
});

But how much effort would we make to do that? And how much effort will we put into maintaining it all?

if(e0.exist()) {
    e0.actionA();
    e0.actionC();
}

Solution 5

If you are performing a simple check on an object and then executing some statements based on the condition then one approach would be to have a Map with a Predicate as key and desired expression as value for example.

Map<Predicate<Integer>,Supplier<String>> ruleMap = new LinkedHashMap <Predicate<Integer>,Supplier<String>>(){{
    put((i)-> i<10,()->"Less than 10!");
    put((i)-> i<100,()->"Less than 100!");
    put((i)-> i<1000,()->"Less than 1000!");
}};

We could later stream the following Map to get the value when the Predicate returns true which could replace all the if/else code

ruleMap.keySet()
       .stream()
       .filter((keyCondition)->keyCondition.test(numItems,version))
       .findFirst()
       .ifPresent((e)-> System.out.print(ruleMap.get(e).get()));

Since we are using findFirst() it is equivalent to if/else if /else if ......

Share:
108,250
yelliver
Author by

yelliver

Updated on October 31, 2021

Comments

  • yelliver
    yelliver over 2 years

    With Java 8, I have this code:

    if(element.exist()){
        // Do something
    }
    

    I want to convert to lambda style,

    element.ifExist(el -> {
        // Do something
    });
    

    with an ifExist method like this:

    public void ifExist(Consumer<Element> consumer) {
        if (exist()) {
            consumer.accept(this);
        }
    }
    

    But now I have else cases to call:

    element.ifExist(el -> {
        // Do something
    }).ifNotExist(el -> {
        // Do something
    });
    

    I can write a similar ifNotExist, and I want they are mutually exclusive (if the exist condition is true, there is no need to check ifNotExist, because sometimes, the exist() method takes so much workload to check), but I always have to check two times. How can I avoid that?

    Maybe the "exist" word make someone misunderstand my idea. You can imagine that I also need some methods:

    ifVisible()
    ifEmpty()
    ifHasAttribute()
    

    Many people said that this is bad idea, but:

    In Java 8 we can use lambda forEach instead of a traditional for loop. In programming for and if are two basic flow controls. If we can use lambda for a for loop, why is using lambda for if bad idea?

    for (Element element : list) {
        element.doSomething();
    }
    
    list.forEach(Element::doSomething);
    

    In Java 8, there's Optional with ifPresent, similar to my idea of ifExist:

    Optional<Elem> element = ...
    element.ifPresent(el -> System.out.println("Present " + el);
    

    And about code maintenance and readability, what do you think if I have the following code with many repeating simple if clauses?

    if (e0.exist()) {
        e0.actionA();
    } else {
        e0.actionB();
    }
    
    if (e1.exist()) {
        e0.actionC();
    }
    
    if (e2.exist()) {
        e2.actionD();
    }
    
    if (e3.exist()) {
        e3.actionB();
    }
    

    Compare to:

    e0.ifExist(Element::actionA).ifNotExist(Element::actionB);
    e1.ifExist(Element::actionC);
    e2.ifExist(Element::actionD);
    e3.ifExist(Element::actionB);
    

    Which is better? And, oops, do you notice that in the traditional if clause code, there's a mistake in:

    if (e1.exist()) {
        e0.actionC(); // Actually e1
    }
    

    I think if we use lambda, we can avoid this mistake!

  • Eran
    Eran about 6 years
    OP wanted to avoid checking the condition twice - if the exist condition is true, no need to check ifNotExist
  • Michael
    Michael about 6 years
    @Eran If it's a boolean flag, I see no reason to bother to over-engineer anything. Still, I've updated my answer.
  • Andrew Tobilko
    Andrew Tobilko about 6 years
    I liked this one. It might be simplified to (exist() ? ifExist : orElse).accept(this);
  • Joop Eggen
    Joop Eggen about 6 years
    @Andrew I did not expect 4 upvotes, as I just wanted to indicate that a custom exists probably points to something that Optional would be more suited for. And forEach definitely is wrong, thanks.
  • Eric Duminil
    Eric Duminil about 6 years
    And with only 45 lines, you managed to save -2 lines. Typical Java :D
  • Michael
    Michael about 6 years
    @EricDuminil Well, the interface and 2 classes are generic and reusable.
  • yelliver
    yelliver about 6 years
    Thank you for your answer, this is good approach, but actually I want to use fluent style, this is not my expectation
  • yelliver
    yelliver about 6 years
    yes, it's exactly that I want to use fluent interface, but with you solution, if I do not check otherwise, I cannot chain other method of element, for ex: element.ifExist(...).otherElementMethod()
  • Andrew Tobilko
    Andrew Tobilko about 6 years
    @yelliver, the body of otherElementMethod should be executed only if an element exists?
  • yelliver
    yelliver about 6 years
    @Andrew maybe the "exist" name make you confused, it can be some condition satisfied, so the otherElementMethod should always executed
  • fps
    fps about 6 years
    There's also Optional.or in Java 9+
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont about 6 years
    A concrete Else that does a runtime check might be more efficient than an abstract one with exactly 2 implementations.
  • jpmc26
    jpmc26 about 6 years
    @yelliver Why do you want a "fluent style"? What problem does it solve? This answer has the virtue of being much simpler and more straightforward and easier to use. You should have a very good reason to complicate things. (Note that this means you need a very good reason not to just use the if/else syntax in the first place.)
  • Michael
    Michael about 6 years
    Why is getOperand().otherElementMethod() better than a second statement element.otherElementMethod() after the conditional stuff?
  • Michael
    Michael about 6 years
    Also don't use Function<E, Boolean>, use a Predicate
  • jpmc26
    jpmc26 about 6 years
    @yelliver I'd like to note that I think this answer fits better with the example you edited in than a fluent interface does.
  • Andrew Tobilko
    Andrew Tobilko about 6 years
    @Michael, It would be nearly the same, but OP wants to form chains. About Predicate - you're right, thanks
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont about 6 years
    @michael Store a boolean. If true, otherwise runs the lambda. If false it discards the lambda.
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont about 6 years
    class Else<T> { private final T item; Else(final T item) { this.item = item; } public void otherwise(Consumer<T> consumer){ if(!item.exists()) consumer.accept(item);}} in ifExists always return Else<>(this).
  • Michael
    Michael about 6 years
    @Yakk Got it. Thanks. See Eran's comment above. He specifically wanted to avoid calling item.exists() twice. Suppose it's running some DB query or something. Also, my Else is generic. Yours would have to be implemented as Else<T extends ExistsProvidingInterface>
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont about 6 years
    @michael then add a bool and check it instead of exists. And oass it to constructor. (T instance only required if you want to oass disengaged optional to lambda, which seems questionable)
  • Nitish Borade
    Nitish Borade over 2 years
    LinkedHashMap should be used instead of HashMap. If i=9, with HashMap you cannot guarantee i < 10 will be evaluated before i < 100, so if you expect "Less than 10!" you could instead get "Less than 100!". With LinkedHashMap, conditions are evaluated in the order you put in the map, so with i=9 will return "Less than 10!"