39

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!

Neuron
  • 5,141
  • 5
  • 38
  • 59
yelliver
  • 5,648
  • 5
  • 34
  • 65
  • 65
    Out of curiosity, just what benefit are you hoping to gain by replacing if-else statements with lambdas? (The only side effects I can think of are extra overhead for the code execution, and just being a saddest to the poor sap who has to maintain this pattern later) – Tezra Apr 16 '18 at 13:23
  • 1
    Note that `int x; if(..){ x++; }` is correct, but `int x; element.ifExists(el -> { x++; });` is not, since the latter can never modify local variables, according to Java rules. So, the lambda variant has more constraints. – chi Apr 16 '18 at 15:22
  • 1
    If you want to produce more lines why don't you add some comments that make things clearer, instead of lambdas that make it less clear ;-) – Droidum Apr 16 '18 at 16:31
  • @Tezra I want to use fluent style, and I also want to know how helpful lambda in java 8. I know there will be extra overhead for code execution, but for example, stream API in java 8 has extra overhead to traditional for loop. I mean in some cases, we can trade off the code execution for other things – yelliver Apr 16 '18 at 18:02
  • 2
    For fluent style, my understanding is that using lambda for that is an anti-pattern (a fluent interface is suppose to some what read like what it is doing. Lambda reads nothing like anything). So far, the only use I've seen for lambda that justifies the maintenance pain is the ability to pass functions as parameters. Otherwise, I have to return to my first comment. "Just what benefit are you hoping to get?" – Tezra Apr 16 '18 at 18:21
  • 1
    At the very least, I would recommend putting the if-else logic in a function over a lambda. If you modeled your domains correctly, normal OOP based objects can implement fluent interfaces perfectly. (Just looks at the examples on the wiki page. Good objects work perfectly fine. https://en.wikipedia.org/wiki/Fluent_interface#Java) – Tezra Apr 16 '18 at 18:29
  • At which point is the existence of `element` first checked? Can you already determine if it exists when the object is created? What about simply caching an "exists" boolean? – Cephalopod Apr 16 '18 at 19:38
  • @Cephalopod I thought about caching boolean, but should we care about multithread if using caching boolean? – yelliver Apr 16 '18 at 19:43
  • 1
    @yelliver keep it in mind, but worst case is you evaluate it twice concurrently. If you can do the check before the object is created you can also use polymorphism. – Cephalopod Apr 16 '18 at 20:05
  • 6
    Just FYI, if I saw you doing this in production code, I'd go have a discussion with our supervisor/manager about your inability to write clean, simple code. Use the `if`/`else` in production. – jpmc26 Apr 16 '18 at 21:47
  • 5
    @jpmc26 you sound like a dream to work with – Michael Apr 16 '18 at 22:05
  • 6
    @Michael Sorry if I've lost all patience with people writing bad code because they were trying to follow some principle or fad. Been trying to get a system like that on the rails for about 6 years, with a few months off where someone drove it *way* further off the rails while I was gone. (No joke, we've had to rewrite nearly everything that was done during that period, the parts that have actually made it to prod, anyway.) So I try to dissuade thinking that's not problem solving based whenever I can, to save as many as possible the heartache. – jpmc26 Apr 16 '18 at 23:13
  • 9
    To echo what some others have said: it’s fine if you’re doing this for practice, but please don’t ever do this in real code. Developers, yourself included, spend a lot more time maintaining existing code than writing new stuff, so it is in your best interest, and ours, to write your code so it is as easy to read as possible. if/else statements are normal and good. – VGR Apr 17 '18 at 00:57
  • @jpmc26 I appreciate your advice, but I think you have a partial evaluation on my idea, I have update my question – yelliver Apr 17 '18 at 03:18
  • 2
    "*do you notice that in the traditional if clause code, there's a mistake in*" it's the reason why properly naming variable is important... not `e0`, `e1`, `etc`... – Andrew T. Apr 17 '18 at 03:37
  • @AndrewT. I am trying to create a base skeleton framework, other developers will use my framework to code, so I think if I provide them the 2nd way, they can avoid that type of mistake (if many e0, e1, e2..., maybe they will copy/paste and lead to mistake) – yelliver Apr 17 '18 at 03:45
  • You might want to look at Category Theory, what you're describing sounds like an operation called "fold" – Morgen Apr 17 '18 at 04:53
  • 1
    Why not ditch Java completely and just start writing everything in pure lambda calculus? Then everything will be a lambda, including objects, functions, control structures and numbers. – Matti Virkkunen Apr 17 '18 at 10:03
  • 1
    `In Java 8 we can use lambda forEach instead of a traditional for loop.` Just because you CAN do something, does not mean you ever should. (PS, never use Lambda foreach in production for the same reason as all upvoted comments) `If(object.condition()){...}` will always be infinitely easier to understand, use, and maintain than `object.condition(var -> )` (you know what I mean!) Code should not reinvent basic language features. Code should provide missing functionality (that a 10 minute google search won't reveal a built in function for) – Tezra Apr 17 '18 at 12:10
  • 3
    Please refer to http://thedailywtf.com for more examples of things the language lets you do, but you never should. (P.S. Just because Java did it, doesn't necessarily mean it was a good idea) – Tezra Apr 17 '18 at 12:10
  • @Tezra. You said "PS, never use Lambda foreach in production". Are you refering to Intstream.range(0, 10).forEach(x -> magic) – GilbertS Apr 07 '20 at 14:11
  • @Tezra a good reason for this question is Sonar code complexity. Indeed, sonar assigns a complexity to each conditional structure. So if you have in a function containing a few dozen of "if" statements Sonar will throw a cognitive complexity issu. Spliting that kind of function is not realy a solution (each subfunction can become more and more complex). So finding an optimized way to perform the "if" statement "lineary" is the real solution in this scenario. But I don't realy know how to do that. And i think your right : lambda is not the solution anymore. May be consumers? – soung Mar 02 '21 at 21:37

5 Answers5

38

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
  • 107,315
  • 7
  • 83
  • 138
  • 1
    @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. – Joop Eggen Apr 16 '18 at 13:39
  • 4
    There's also `Optional.or` in Java 9+ – fps Apr 16 '18 at 19:45
21

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
});
Michael
  • 41,989
  • 11
  • 82
  • 128
  • 2
    OP wanted to avoid checking the condition twice - `if the exist condition is true, no need to check ifNotExist` – Eran Apr 16 '18 at 12:28
  • 1
    @Eran If it's a boolean flag, I see no reason to bother to over-engineer anything. Still, I've updated my answer. – Michael Apr 16 '18 at 12:31
  • 32
    And with only 45 lines, you managed to save -2 lines. Typical Java :D – Eric Duminil Apr 16 '18 at 14:38
  • 6
    @EricDuminil Well, the interface and 2 classes are generic and reusable. – Michael Apr 16 '18 at 14:52
  • 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() – yelliver Apr 16 '18 at 18:14
  • @yelliver, the body of `otherElementMethod` should be executed only if an element exists? – Andrew Tobilko Apr 16 '18 at 18:33
  • @Andrew maybe the "exist" name make you confused, it can be some condition satisfied, so the otherElementMethod should always executed – yelliver Apr 16 '18 at 18:44
  • A concrete `Else` that does a runtime check might be more efficient than an abstract one with exactly 2 implementations. – Yakk - Adam Nevraumont Apr 16 '18 at 20:10
  • @michael Store a boolean. If true, otherwise runs the lambda. If false it discards the lambda. – Yakk - Adam Nevraumont Apr 17 '18 at 11:15
  • `class Else { private final T item; Else(final T item) { this.item = item; } public void otherwise(Consumer consumer){ if(!item.exists()) consumer.accept(item);}}` in `ifExists` always return `Else<>(this)`. – Yakk - Adam Nevraumont Apr 17 '18 at 11:32
  • @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` – Michael Apr 17 '18 at 11:36
  • @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) – Yakk - Adam Nevraumont Apr 17 '18 at 12:01
12

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
  });
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Eran
  • 387,369
  • 54
  • 702
  • 768
  • 1
    I liked this one. It might be simplified to `(exist() ? ifExist : orElse).accept(this);` – Andrew Tobilko Apr 16 '18 at 12:45
  • Thank you for your answer, this is good approach, but actually I want to use fluent style, this is not my expectation – yelliver Apr 16 '18 at 18:11
  • 3
    @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 Apr 16 '18 at 21:50
  • @yelliver I'd like to note that I think this answer fits better with the example you edited in than a fluent interface does. – jpmc26 Apr 17 '18 at 03:40
5

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();
}
Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
3

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

ag157
  • 85
  • 1
  • 7
  • 2
    `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!"` – Nitish Borade Oct 11 '21 at 17:04