Java 8 ifPresent vs ternary operator

Question

What do you think is better (with arguments, of course):

Optional.ofNullable( userName )
      .ifPresent( nonNullUserName  -> header.setUser( createUser( nonNullUserName ) ) );

or

header.setUser( userName == null ? createUser( userName ) : null );

The method createUser creates xml element and the intent of the whole peace of code is to set it in a SOAP request depending on presence of userName.

The benefits of the first approach I see is the absence of useless operations, the code does one thing and no more. But the second approach lets you save one more line of code thus appearing to be more laconic.

UPDATE: I guess I missed a thing I actually implied and it caused certain misunderstanding. It would be better to provide cleaner examples with explanations if you have some.

Solution 1

Optional was designed to be used as a return type for methods that can't always return a value, like for example getFirstElementOfAList(), which can't return anything else if the list is empty: returning null would be less clear than returning an empty Optional, and could mean that the first element is null, and not that there is no first element.

The designers don't intend Optional to be used as a convenience construct to chain methods. See the presentation by Stuart Marks (who works on the JDK), and especially what he says at the 28th minute:

It's generally a bad idea to create an Optional for the specific purpose of chaining methods [...]

You're of course free to disrespect the intended design and the best practices, but your colleagues might also be free to hate you for that :-)

Note that the two snippets you posted don't do the same thing. The first one does nothing if the user name is null, whereas the second one sets the user to null (which might in turn throw an exception or overwrite the previous value)

I would just use (assuming what you want is the first version):

if (userName != null) { 
    header.setUser(createUser(userName)); 
}

which is extremely clear, doesn't abuse Optional, and doesn't create an Optional instance just to chain methods.

Solution 2

They are different things, one is an Object, another is an operator. you shouldn't comparing between them. the first approach can be simplified to this, which will be more readable & descriablable:

Optional.ofNullable(userName).map(this::createUser).ifPresent(header::setUser);

IF you really want to comparing between them, the only one different is you have been mentioned above. and if you process condition or result more complex in an operator that will result to the expression is too complex to read.

Comments

[JB Nizet]

They don't even do the same thing. But anyway, Optional is not designed to do that, and is of course a tiny bit less efficient. Optional is designed to be a return type of a method. I would just use if (userName != null) { header.setUser(createUser(userName)); } (assuming you want to do what the first snippet does, and not what the second does).

[Kayaman]

Agreed. The question is flawed as it presents 2 bad ways to do something extremely simple. Talking about saving lines of code is a futile argument when we're dealing with Java, which is a verbose language in many other ways.

[Dmitry Senkovich]

@ JB Nizet Interesting approach. But our code style doesn't let write it in one line so I would say the first approach above and the first answer look cleaner to me. Thanks for the idea

[Dmitry Senkovich]

@Kayaman Well, I wouldn't say our two approaches are the only one. That is why I'm here. Can't understand your hate

[JB Nizet]

The point is not to write it on one line. The comment system doesn't allow multiple lines. The point is to use a simple if block. You shouldn't care if it's on one line or on three.

[Dmitry Senkovich]

@JB Nizet Of course, agreed. But to my mind the first approach looks cleaner, so I would like to know the opinion of the community. But I agree, there's nothing in count of lines of code

[JB Nizet]

To have the official point of view of the designers of Optional, working on the JDK, see youtube.com/watch?v=Ej0sss6cq14 (28th minute). The slide says: It's generally a bad idea to create an Optional for the specific purpose of chaining methods [...]

[holi-java]

in short, Optional is designed to avoid NullPointerException in jdk-8.

[Dmitry Senkovich]

@JB Nizet This sounds like an answer to my question. Could you please anwer it? It might be helpful

[Kayaman]

If you think it's hate to point out your question is flawed, then you don't really understand what hate means.

[Kayaman]

@holi-java That's not true. Optional doesn't remove null checks, unless you write hard to understand code (like in the question) to explicitly try to use Optional in every possible place.

[holi-java]

@Kayaman in the question above, it is.

[Kayaman]

@holi-java I'm not talking about the question. I'm talking about your broad statement of "Optional is designed to avoid NullPointerException in jdk-8". It can help avoid null checks, but your statement is not true.

[holi-java]

@Kayaman thanks for your feedback, sir. my comments is to the OP, not to everybody, I'm sorry.

[Dmitry Senkovich]

Thanks, I really like your suggestion. Gonna see what others think

[Dmitry Senkovich]

Well, finally JB Nizet has found the official point of view on the problem. Such constructions as I proposed in the question are not the cases Optional was added for.

[holi-java]

plus one, sir, I agree on, but I want to show the OP that Optional can also appearing to be more laconic and in my answer I have been described.

[holi-java]

@DmitrySenkovich Hi, what I want to tell you is Optional can also appearing to be more laconic and in my answer I have been described. and I don't suggest you must do with Optional chain. and in my answer I only tell you needn't comparing two different things.

[Dmitry Senkovich]

I agree. I also see it as more laconic. No sure I understand you thoughts about comparing it. If I can use two approaches I always can compare it somehow I think. Here I wanted to see the community opinion because we had a dispute in our team.

[holi-java]

@DmitrySenkovich well. you should do the first approach in your mind, and after if you find the best one, just replacing the oldest one. if you do everything thinking how and why at first? you will cost many time choosing between them.

[Dmitry Senkovich]

Not sure I understand you. Everyone makes mistakes and fixes it if he wants and can do it. Everything evolve, today one approach is better and the day after another one. Programming is all about not doing thing only but evolving also I think

[holi-java]

@DmitrySenkovich To be honest, my English is very bad， If I always thinking how to answer & describe my mind by English, I couldn't answer any question. first I write a broad-brush answer, and then I refactoring my answer, and so on.

[fps]

Is there any valid, scientific, or at least empiric argument to not use Optional to chain methods (as in @holi-java 's answer) besides the intentions of the JDK team? Because, well, I had the intention to be a rockstar, but ended up programming in Java...

[JB Nizet]

It's less efficient, creates useless objects, and, in this case at least, is less clear than a simple if statement.

[fps]

Honestly, I fail to see the problem with this approach. Actaully, I find it much better than an imperative if construct.

[fps]

I get your point, but regarding efficiency, we're talking about a few null checks and a wrapper class => negligible. Regarding useless objects, most people use a framework, i.e. Spring, JSF, etc. That is a few thousand of real, useless objects. Regarding how clear it is, it's just a matter of opinion. The other answer is crystal clear to me. My point is against the statement someone from Oracle created this to be used this way. That is no valid reason.

[holi-java]

@FedericoPeraltaSchaffner thanks for your feedback, but they are talking on the performance on this question.

[holi-java]

@JBNizet @FedericoPeraltaSchaffner Hi, but sometime maybe create Optional chain more great where Optional.map return in the method like as getFirstElementOfAList but you can't control, so you used Optional.flatMap in your code. sir, am I right?

[fps]

@holi-java yes, when used as a return value, Optional is very useful. It is expressive, because it clearly states that the value can be present or not. And it is also very handy, because you just use Optional's operations to transform the result and consume it.

[Stuart Marks]

Thanks for referencing my talk. :-)

[Stuart Marks]

@FedericoPeraltaSchaffner As the guy from Oracle being cited, this isn't about "Somebody from Oracle said not to so you shouldn't do it." Rather, it's about design intent and effectiveness. If something is designed for a purpose, it's likely to be useful for that purpose. It's less likely to be useful for purposes for which it wasn't designed. In particular, Optional wasn't designed to replace if-statements. In this case, the alternative using Optional might be crystal clear. (I don't happen to think so, but reasonable people can disagree on this.)

[Stuart Marks]

@FedericoPeraltaSchaffner However, when examples get more complicated, such as the cases I cite in my talk (which are originally from Stack Overflow) I think you'll find that using Optional this way starts to break down quickly. People can get really clever -- too clever -- at trying to replace control structures with Optional. This tends to lead to obscure, hard-to-understand code. Hence I recommend avoiding using Optional this way.

[fps]

@StuartMarks Hey, thanks for answering my comment. You are giving arguments, and that was precisely my point. The argument shouldnt be "because Stuart Marks said this in a conference", but what you have actually said instead. That was my whole point.

[fps]

@StuartMarks And I agree with you: in this case replacing if with Optional is easy, but in more complex cases using Optional produces code that makes you have nightmares. Same for streams, I think. Again, thank you very much for taking the time to participate and make your point crystal clear. Btw, I have upvoted this answer :)