16

Hello i have two samples of code

if/else if/else statements

private Object getObj(message) {
        if (message.getA() != null)
            return message.getA();
        else if (message.getB() != null)
            return message.getB();
        else if (message.getC() != null)
            return message.getC();
        else return null;
}

Optional statements

private Optional<Object> wrap(Object o){
    return Optional.ofNullable(o);
}

private Object getObj(message) {
    return  wrap(message.getA())
            .orElseGet(() -> wrap(message.getB())
            .orElseGet(() -> wrap(message.getC())
            .orElse(null)));
}

So my question is how these two compare in terms of performance (i have about 15-20 if-else statements on actual code)?

Is it worth refactoring the code readability vs performance or is a misusage of optionals?

Also what is the performance penalty in case the if/else-if statements grown to 100+?

Thanks in advance

Panos K
  • 1,061
  • 12
  • 26
  • 7
    I personally find the chaining easier to read - but my colleagues think I'm weird, so _it depends_ I guess. performance wise, this is 99% irrelevant for you; unless a profiler proves otherwise – Eugene May 21 '19 at 09:31
  • 2
    _Is it worth refactoring the code readability vs performance or is a misusage of optionals?_ It depends on your needs – David Pérez Cabrera May 21 '19 at 09:31
  • 1
    You care about performance of 100+ checks. I would rather worry about readability of of 100+ if/else statements. – Arnaud Denoyelle May 21 '19 at 09:32
  • How about an approach of creating `Stream`s of all these `Optional`s and returning using `findFirst` with an `isPresent` check? – Naman May 21 '19 at 09:33
  • or a `Stream` without `Optional`... but you will probably soon end up using a `Stream` with a `Supplier` then... don't know whether that's really worth it... I think the `if/else`-statements are way more readable... or at least you immediately know *why* you are returning it, whereas with the `Optional` it is rather hidden... not to mention that it's basically an abuse of the `Optional` itself... – Roland May 21 '19 at 09:39
  • 4
    If you don’t know what [premature optimization](https://stackify.com/premature-optimization-evil/) is, please look it up. – Ole V.V. May 21 '19 at 09:46
  • 2
    FYI Java 9 has ```Optional#or``` method to avoid the ```wrap``` you are doing. – wilmol May 22 '19 at 02:13
  • 2
    @wilmol `or` is nice but doesn’t save you from `wrap` in this case. The syntax would be `return wrap(message.getA()) .or(() -> wrap(message.getB())) .or(() -> wrap(message.getC())) .orElse(null);`. Brackets are placed a little differently. `wrap` is used the same. – Ole V.V. May 22 '19 at 03:03
  • maybe the getters should return `Optional` (second line of Lino's answer) - we also do not know how expensive the call of the getters is or even its side effects (reason against above if-else solution) – user85421 May 26 '19 at 05:28

5 Answers5

32

Don't use Optionals for conditional logic.

They were designed, to be returned from a method to indicate a potentially absent value.

Just because you can nicely chain them into a single line doesn't mean that it's understandable. Also you literally gain nothing. The performance overhead may be significant. In the worst case N objects being created and then discarded. Just stay with your "normal" if-else chains.

Instead of finding ways to make your current code more readable, take a step back and ask yourself why you need 15-20 if-else statements. Can you split some logic up? Why do you need a getter for so many different fields with potentially different types in the first place? etc.

Lino
  • 19,604
  • 6
  • 47
  • 65
  • Thanks for the input Lino, actually this is a core validation method that needs to make all these checks (I know its probably a spliting responsibilities issue but at the moment refactoring logic is very tedious) – Panos K May 21 '19 at 09:38
  • @PanosK If you would maybe include more specific reasons in your questions why you need do this (call site, examples etc.), then I may give a more concrete answer. – Lino May 21 '19 at 09:41
  • 2
    I guess i got my answer "Don't use Optionals for conditional logic" , i will stick with if/else and try to refactor logic to reduce them in future. Thanks Lino (Still dont get the downvotes on my question). – Panos K May 21 '19 at 09:45
5

There is a third form (allowing still some variation).

return Stream.<Supplier<Object>>of(message::getA, message::getB, message::getC)
        .map(Supplier::get)
        .filter(Objects::nonNull)
        .findFirst()
        .orElse(null);

Probably the least flexible and efficient at this moment, but clear.

Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
Joop Eggen
  • 107,315
  • 7
  • 83
  • 138
  • 3
    You will have to write `Stream.>of(…)` to make this compile. – Didier L May 21 '19 at 23:06
  • 2
    I note that the use of suppliers in the stream makes the evaluation of this just as lazy as in the question: `getB` is only called if `getA` returned `null`, etc. Like. – Ole V.V. May 22 '19 at 07:50
5

tl;dr

If your goal is condensed code, then use ternary chaining. Performance is likely identical to that of a series of if-then-else statements.

        ( this.getA() != null ) ? this.getA()
                : ( this.getB() != null ) ? this.getB()
                : ( this.getC() != null ) ? this.getC()
                : null;

Ternary chaining

As the Answer by Lino correctly states, you are trying to take Optional beyond their original design purpose (returning values within lambdas & streams). Generally best to use Optional only with a return statement, and only then when you want to make clear that null is a valid value to be returned. See this Answer by Brian Goetz.

A ternary operator is a condensed if-then-else, combined into a one-liner.

result = test ? valueToUseIfTestIsTrue : valueToUseIfTestIsFalse

Example:

Color color = isPrinterMonochrome ? Color.GREY : Color.GREEN ;

Use a chain of ternary statements.

So this:

    if ( this.getA() != null )
        return this.getA();
    else if ( this.getB() != null )
        return this.getB();
    else if ( this.getC() != null )
        return this.getC();
    else return null;

…becomes this:

    return
            ( this.getA() != null ) ? this.getA()
                    : ( this.getB() != null ) ? this.getB()
                    : ( this.getC() != null ) ? this.getC()
                    : null;

Example code.

public String getA ()
{
    // return "A";
    return null;
}

public String getB ()
{
    // return "B";
    return null;
}

public String getC ()
{
    return "C";
    // return null;
}

public String getABC ()
{
    if ( this.getA() != null )
        return this.getA();
    else if ( this.getB() != null )
        return this.getB();
    else if ( this.getC() != null )
        return this.getC();
    else return null;
}

public String getABCTernary ()
{
    return
            ( this.getA() != null ) ? this.getA()
                    : ( this.getB() != null ) ? this.getB()
                    : ( this.getC() != null ) ? this.getC()
                    : null;
}

Run that example code.

String s = this.getABCTernary();
System.out.println( "s: " + s );

C

Pros and cons

  • The upside to the ternary chain is condensed code, collapsed into a one-liner.
  • The downside is that you are calling your getter method twice in this particular situation just to get a single value. Not a problem for a simple fetch-the-variable kind of getter, but impact performance if the getter is a time-consuming method such as a remote web services call. And, the cascading if-then-else has the same problem, also calling your getter twice.

Performance

how these two compare in terms of performance

The ternary operator in Java is "short-circuiting", meaning the left or right side that matches the test results is the only code called. In our code here, if getA returns a non-null value, that value is returned immediately. The further calls to getB and getC are never executed. So in this regard, the performance of the chained ternary is the same as a cascading if-then-else statement: first-match wins, no further calls.

If you mean performance as in nanoseconds of execution, I do not know. Worrying about that would be falling into the trap of premature optimization. Modern JVMs are extremely well-tuned for optimizing your code.

Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
5

In my opinion after around 20 years of commercial experience, I've formed a view that pursuing readability is absolute stupidity, and at the same time, intentionally writing convoluted code is evil.

I know this goes totally against popular opinion.

However, everyone needs to realize this...

  1. What is readable to one person is not necessarily readable to the next. Even in this thread, we have varying opinions of whether if or Optional are more readable or not. These kinds of debates will occur irrespective of what constructs or situations we are in.
  2. If we take the if option, which is more performant than the functional approach, each and every time, then the people reading that code will get used to it and find it MORE READABLE - because it is the style that they have now become accustomed to.
  3. Performant code does not have to be "hard to read"... but this loops back to points 1 and 2. It's a matter for developers to actually know the fundamentals of the language they are using, and writing the code appropriate for that language, rather than trying to form "english sentences" in their code.

So, in essence: go with the if... do NOT use that Optional!

Lino
  • 19,604
  • 6
  • 47
  • 65
John
  • 800
  • 5
  • 11
2

A couple of days ago I ran a thorough performance analysis. There's a huge performance impact. With AdoptOpenJDK, if statements are up to 10 times faster. When the JIT compiler runs hot, this reduces to a 20% penalty.

GraalVM does a better job: 3 times slowdown with the cold JVM, and after giving the compiler enough time to do its magic, there's also a 20% performance penalty.

However, the real question is which version is better for reading and maintaining the application. If you're like me, it's easier to read the if statement, but there are also people preferring the functional approach.

If you're ready for a deep, deep dive, I invite you to read my detailed analysis about the performance and the implementation of Optional.orElseGet() and its friends.

Stephan Rauh
  • 3,069
  • 2
  • 18
  • 37