18

Which null-check is preferable?

Optional.ofNullable(port).ifPresent(settings::setPort);

or

if (port != null) {
   settings.setPort(port);
}
Nikolas Charalambidis
  • 40,893
  • 16
  • 117
  • 183
applejuice
  • 203
  • 1
  • 2
  • 5
  • Preferable for whom? This is an opinion based question. Which one creates more overhead, for the computer and the mind of the next person to read the code? – M. le Rutte Aug 27 '18 at 11:37
  • 3
    This is opinion-based. But I prefer the second option. More readable. Additional, `Optional` has another purpose. – Seelenvirtuose Aug 27 '18 at 11:37
  • 1
    Just for an `if`, I would not, at least for a use of `orElse`, this become a bit more interesting. – AxelH Aug 27 '18 at 12:00
  • 4
    @Kayaman that’s not so unusual. Especially as some of these points may interact. – Holger Aug 27 '18 at 12:10

3 Answers3

31

In Java, an Optional value is a fusion of a bit that indicates presence or absence, with a value of an arbitrary reference type T or a primitive int, long, or double.

Fusing these is especially useful when returning a value from a method, as methods have only a single return value. It's often necessary to use a special value such as null in the case of reference types, or -1 in the case of int, as a sentinel to indicate the "no-value" case. Using Optional as a return value avoids the problem of the caller accidentally misusing the sentinel value as the real return value.

Given this, line of code such as

Optional.ofNullable(port).ifPresent(settings::setPort);

is strange in that it fuses a value with the present/absent bit in the first part of the line and then immediately separates them in the second part of the line. This adds complexity to what is ultimately a fairly simple task: checking whether port is non-null and conditionally performing some action. The alternative code snippet:

if (port != null) {
    settings.setPort(port);
}

expresses quite clearly exactly what it does.

It's true that the if-statement takes more vertical space than the Optional chain. The Optional chain is denser, but it's also harder to understand: a poor tradeoff.

Stuart Marks
  • 127,867
  • 37
  • 205
  • 259
  • This is the only correct answer given here. It deserves more +1s. Java8 in action promotes `Optional.ofNullable(port).ifPresent(settings::setPort);` kind of use of `Optional`. But `Optional` have severe performance issues according to some books. So it should only be used as a return value from a function. – Ravindra Ranwala Aug 28 '18 at 02:25
  • 1
    @RavindraRanwala I totally disagree with this - we use this pattern in our code, not much, but we do. The thing is, is that for *our* team - apparently this is a lot easier to read (myself included). – Eugene Oct 28 '18 at 04:07
  • Optional is syntactic sugar and comes at its own cost. for simple null checks, better use traditional null checks – Tukaram Bhosale Oct 02 '20 at 08:57
  • one more point here is additional objects created (though minor) when there is no real need for it. 1+ – Eugene Sep 19 '22 at 08:28
7

Although, the snippet you have posted in the question is just a simple way to avoid the ugly null-check, yet is valid, correct and null-safe. Follow your personal preference in this case.

The real power of Optional are the following methods:

As example, let's say you want to get another value from port to add to the list and avoid the NPE if port is null:

Optional.ofNullable(port).map(port::getSomeValue).ifPresent(settings::setPort);

Moreover, please, avoid the following meaningless substitution of null-check I see often:

if (Optional.ofNullable(port).isPresent()) {
    settings.setPort(port);
}
Nikolas Charalambidis
  • 40,893
  • 16
  • 117
  • 183
  • 5
    `.map(Port::getSomeValue)` to keep it simple. You really see the last point often ? That's sad ! – AxelH Aug 27 '18 at 12:06
2

It is dependent on several factors when to use this. If port is a property within the class, probably using Optional is a bit overkill. (and don't useOptionals as property as they are not serializbie)

I think Optionals are great when for example writing a library. 

public Optional<Integer> getPort()

Is much more descriptive for other developers then

// Returns null if not set
public Integer getPort()
Albert Bos
  • 2,012
  • 1
  • 15
  • 26
  • Returning `null` is usually implied, so that wouldn't really warrant a comment nor would it be surprising to get `null` back. `Optional` has its places, but it's not an automatic replacement for methods that may return `null`. – Kayaman Aug 27 '18 at 11:56
  • @Kayaman Maybe my example is oversimplified also because of "simple" get method. Like anything else use it where it makes sense. But there might be methods where it is not so implied, or parameters to make it more clear. – Albert Bos Aug 27 '18 at 12:15
  • 1
    Indeed, but like this question shows, some people think `Optional` was a replacement for `null`, which it isn't. Kind of like sopme people think streams are a replacement for normal loops, and you have questions for "improving" code by replacing loops with streams. – Kayaman Aug 27 '18 at 12:18