By yelliver


2018-04-16 12:21:48 8 Comments

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!

5 comments

@Andrew 2018-04-16 12:37:59

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();
}

@Michael 2018-04-16 22:09:21

Why is getOperand().otherElementMethod() better than a second statement element.otherElementMethod() after the conditional stuff?

@Michael 2018-04-16 22:10:57

Also don't use Function<E, Boolean>, use a Predicate

@Andrew 2018-04-17 08:33:32

@Michael, It would be nearly the same, but OP wants to form chains. About Predicate - you're right, thanks

@Eran 2018-04-16 12:24:48

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
  });

@Andrew 2018-04-16 12:45:11

I liked this one. It might be simplified to (exist() ? ifExist : orElse).accept(this);

@yelliver 2018-04-16 18:11:05

Thank you for your answer, this is good approach, but actually I want to use fluent style, this is not my expectation

@jpmc26 2018-04-16 21:50:36

@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.)

@jpmc26 2018-04-17 03:40:25

@yelliver I'd like to note that I think this answer fits better with the example you edited in than a fluent interface does.

@yelliver 2018-04-17 04:39:25

As some suggestions, I have an idea of caching the boolean value:

public class Element {
    private Boolean exist;

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

    public void ifNotExist(Consumer<Element> consumer) {
        if (exist == null) {
            exist = exist();
        }
        if (!exist) {
            consumer.accept(this);
        }
    }
}

@Andrew 2018-04-17 09:24:53

my idea is very close to this - lazy loading and caching the result of a condition. Please, have a look at my 2 updates.

@Sean Burton 2018-04-17 12:02:49

Bear in mind that this approach might not work too well if you introduce multithreading into your application.

@Andrew 2018-04-17 12:13:35

@SeanBurton, are we talking about multithreading here?

@Sean Burton 2018-04-18 10:14:56

Are we not? Nowhere does the OP specifically mention their code is single-threaded, so it is unsafe to assume that it is.

@yelliver 2018-04-18 11:54:20

@SeanBurton yes, may I convert it into AtomicBoolean for thread safe enough?

@Sean Burton 2018-04-18 11:58:48

Making the boolean atomic is not sufficient to make this thread-safe.

@Sean Burton 2018-04-18 12:43:15

To add to that: to make this thread-safe you would need to synchronise around the entire chain of method calls, and I do not know of any easy way of doing that.

@Joop Eggen 2018-04-16 12:38:21

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.

@Joop Eggen 2018-04-16 13:39:24

@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.

@Oleksandr 2018-04-16 17:52:16

ifPresentOrElse should be on the top :)

@Federico Peralta Schaffner 2018-04-16 19:45:28

There's also Optional.or in Java 9+

@Michael 2018-04-16 12:24:33

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
});

@Eran 2018-04-16 12:28:38

OP wanted to avoid checking the condition twice - if the exist condition is true, no need to check ifNotExist

@Michael 2018-04-16 12:31:57

@Eran If it's a boolean flag, I see no reason to bother to over-engineer anything. Still, I've updated my answer.

@Eric Duminil 2018-04-16 14:38:18

And with only 45 lines, you managed to save -2 lines. Typical Java :D

@Michael 2018-04-16 14:52:09

@EricDuminil Well, the interface and 2 classes are generic and reusable.

@yelliver 2018-04-16 18:14:51

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 2018-04-16 18:33:22

@yelliver, the body of otherElementMethod should be executed only if an element exists?

@yelliver 2018-04-16 18:44:09

@Andrew maybe the "exist" name make you confused, it can be some condition satisfied, so the otherElementMethod should always executed

@Yakk 2018-04-16 20:10:09

A concrete Else that does a runtime check might be more efficient than an abstract one with exactly 2 implementations.

@Yakk 2018-04-17 11:15:34

@michael Store a boolean. If true, otherwise runs the lambda. If false it discards the lambda.

@Yakk 2018-04-17 11:32:40

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 2018-04-17 11:36:04

@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 2018-04-17 12:01:04

@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)

Related Questions

Sponsored Content

48 Answered Questions

[SOLVED] Does finally always execute in Java?

57 Answered Questions

[SOLVED] How do I generate random integers within a specific range in Java?

  • 2008-12-12 18:20:57
  • user42155
  • 3377410 View
  • 2790 Score
  • 57 Answer
  • Tags:   java random integer

79 Answered Questions

[SOLVED] Is Java "pass-by-reference" or "pass-by-value"?

14 Answered Questions

[SOLVED] Java 8 List<V> into Map<K, V>

29 Answered Questions

26 Answered Questions

[SOLVED] Why are Python lambdas useful?

30 Answered Questions

[SOLVED] How do I convert a String to an int in Java?

26 Answered Questions

51 Answered Questions

[SOLVED] Creating a memory leak with Java

8 Answered Questions

[SOLVED] What is a lambda expression in C++11?

Sponsored Content