19

Consider the usage of this expression:

String hi = Optional.ofNullable(sayHi()).orElse("-");

which effectively corresponds to this ternary expression:

String hi = sayHi() != null ? sayHi() : "-";

Is this usage of Optional.ofNullable with a method call a good practice? Or just extra verbose coding?


I recognise that Optional.ofNullable actually creates a variable and avoids calling the sayHi() method twice. To avoid this problem you actually could create an extra variable but this adds to the verbosity of the ternary option:

String hi = sayHi();
hi = hi != null ? hi : "-";

On the other hand Optional.ofNullable creates in case of hi not being null an extra Optional object. So there is for sure more overhead to it.

So there seem to be some pros and cons to using this type of construct to replace the ternary constructor.


By the way: this is the Java 8 implementation of Optional.ofNullable:

public static <T> Optional<T> ofNullable(T value) {
    return value == null ? empty() : of(value);
}
Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
gil.fernandes
  • 12,978
  • 5
  • 63
  • 76

6 Answers6

24

In JDK 9 or later, use this:

String hi = Objects.requireNonNullElse(sayHi(), "-");

This avoids having to repeat sayHi() if a ternary operator is used, or to assign its value to a local variable that is reused within the ternary. It might be a small improvement. It also sidesteps the question of whether to use Optional. :-)

Stuart Marks
  • 127,867
  • 37
  • 205
  • 259
16

Whenever I come to think of using the Optional API for a specific purpose I always remind my self of what it was intended to do and why it was brought into the JDK and i.e.

Optional in intended to provide a limited mechanism for library method return types where there is a clear need to represent “no result” and where using null for this is overwhelmingly likely to cause errors - Stuart Marks

Optional is primarily focused on a return type that might or might not have a return value.

Over using this construct like in this specific example of yours just causes extra memory allocation and GC overhead.

I’d keep things simple and instead do:

String hi = sayHi();
if(hi == null) hi = “-“;
...
Ousmane D.
  • 54,915
  • 8
  • 91
  • 126
8

Is this usage of Optional.ofNullable with a method call a good practice?

Conceptually, it's a bad practice. The basic idea is to represent the absence of a return value, not to wrap everything that might be null. I am strongly against this usage.

Or just extra verbose coding?

It looks to me like a failed attempt to make your code more fashionable. ("Look, we are using brand-new Optional from Java 8!")

I prefer readability and clarity over conciseness.

This Optinal usage doesn't provide clarity but raises questions:

Why do you wrap the variable?
What are you going to do with this Optional?
Will it be used/returned below?

It doesn't give brevity either: your second line is even shorter.

To avoid this problem you actually could create an extra variable but this adds to the verbosity of the ternary option.

You aren't creating an extra variable. The one-line version could be:

String hi = (hi = sayHi()) != null ? hi : "-";

Though, your two-line suggestion is absolutely fine:

String messageContent = sayHi();
String hi = messageContent != null ? messageContent : "-";
Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
6

If you're going to allow Optional into your workflow then you should consider modifying the sayHi() method so that it returns Optional<String> which will make the result more elegant:

hi = sayHi().orElse("-");

If you don't want to introduce Optional into your workflow (because it does create an additional object to contain the optional value) then you're better off sticking with simple null checks and the ternary operator.

With regard to the performance costs of Optional (such as increased garbage collection), you'd need to profile your application and decide whether or not this was a real concern.

Additionally, if you're mostly interested in String then be aware of the Objects.toString(Object, String) method which return a default value if the object is null:

hi = Objects.toString(hi, "-");

This is tidier and more elegant than writing out the code manually.

Bobulous
  • 12,967
  • 4
  • 37
  • 68
  • 1
    Thank you for the suggestion of `Objects.toString`. That seems indeed much better for strings. I have no control over the methods which are being called on another note, so I cannot have `Optional` returned. – gil.fernandes Nov 27 '18 at 17:21
1

In brief: avoid the Optional type. The main argument for Optional on return types is, "It's too easy for clients to forget to handle the possibility of a null return value. Optional is ugly and in your face, so a client is less likely to forget to handle the possibility of an empty return value. Everywhere else, programmers should continue to use normal references, which might be null."

I wrote a discussion of the pros and cons of using Java's Optional type: Nothing is better than the Optional type.

mernst
  • 7,437
  • 30
  • 45
0

I think it's mostly a matter of personal taste.

From my point of view it would be more meaningful to just return an Optional<String> and let the caller deal with a missing value.

Optional, being a monadic type, can be leveraged for more then just getting an alt-value.

On the other end your operator seems terse enough, if used consistently it could be practical.

I don't think performance is a big issue here.

minus
  • 2,646
  • 15
  • 18