7

As an example I have an optional like this:

Optional<Desktop> opt = Optional.ofNullable(status).map(Status::getDesktop);

I want to have the desktop and work with it outside of lambda expressions. I do it like this:

if (opt.isPresent()){
    Desktop desktop = opt.get();
    ...
}

Is there a better solution to get the desktop; something like this ?

Desktop desktop = Optional.ofNullable(status).map(Status::getDesktop).ifPresent(get());

EDIT: OrElse was the method I was looking for :)

Tobias Hahn
  • 95
  • 1
  • 1
  • 7
  • 2
    What result do you want `desktop` to hold if it *isn't* present? Have you considered `.orElse`? – Oliver Charlesworth Dec 15 '17 at 14:56
  • You could try `Desktop desktop = Optional.ofNullable(status).map(Status::getDesktop).orElse( defaultValue );` where `defaultValue` could be `null` or anything else that can be assigned to `desktop`, depending on what you need. – Thomas Dec 15 '17 at 14:58
  • `ifPresent` is the clean way to do this (where you only have side effects and aren't using the value to produce another value). If you don't like lambdas or method references, then `isPresent` + `get` is the most simple procedural alternative possible, so it's not clear what you don't like about it. If you know it's present, don't use `Optional`. If you're sure it's present or are ok with a `NoSuchElementException` if it's not, omit the `isPresent`. – Mark Peters Dec 15 '17 at 15:01

2 Answers2

8

If you have a default value for your Desktop, you could try with Optional.orElse:

Desktop defaultDesktop = ...;

Desktop desktop = Optional.ofNullable(status)
    .map(Status::getDesktop)
    .orElse(defaultDesktop);

However, you don't have to necessarily work inside a lambda expression with Optional.ifPresent. You could perfectly use a method that receives a Desktop instance, which would act as the Consumer argument of Optional.ifPresent:

Desktop desktop = Optional.ofNullable(status)
    .map(Status::getDesktop)
    .ifPresent(this::workWithDesktop);

Then:

void workWithDesktop(Desktop desktop) {
    // do whatever you need to do with your desktop
}

If you need additional arguments (apart from the desktop itself), you could use a lambda expression that invokes the method instead:

String arg1 = "hello";
int arg2 = 10;

Desktop desktop = Optional.ofNullable(status)
    .map(Status::getDesktop)
    .ifPresent(desktop -> this.workWithDesktop(desktop, arg1, arg2));

And then:

void workWithDesktop(Desktop desktop, String arg1, int arg2) {
    // do whatever you need to do with your desktop, arg1 and arg2
}
fps
  • 33,623
  • 8
  • 55
  • 110
  • Technically speaking, using a method reference is still considered as "being inside the lambda". It is the same as `.ifPresent(desktop -> { /* method body here */ });`. – Sync Dec 15 '17 at 15:12
5

I was told here just last week that it's a code smell to even use Optional like this at all, so

Desktop desktop = (status != null)? status.getDesktop() : null;
daniu
  • 14,137
  • 4
  • 32
  • 53
  • I wouldn't call it a code smell. If there are two one-liners that have the same input and output, then it's a stylistic or performance choice, but not a design choice. Obviously the ideal would be for a null-safe navigation operator, but without it you just need to pick your poison. – Mark Peters Dec 15 '17 at 15:19
  • @MarkPeters yeah I agree, don't really find it a smell myself either, especially when chained multiple times. Also about the nullsafe operator, one of C#'s nice features. – daniu Dec 15 '17 at 15:55
  • 1
    It’s not as pungent as some other code smells, but it is a code smell. Optional is not a universal replacement for if/else (or the equivalent ternary operator). – VGR Dec 15 '17 at 16:19
  • @VGR: "[Code smell](https://en.wikipedia.org/wiki/Code_smell), also known as bad smell, in computer programming code, refers to any symptom in the source code of a program that possibly indicates a deeper problem. " Here it's just choosing what one liner you want to use for something, and sure there are arguments either way but they don't go past that one line of code. If it's a smell, what's the deeper problem? – Mark Peters Dec 15 '17 at 17:01
  • @MarkPeters Wikipedia and formal definitions aren’t especially meaningful here. It’s called a code smell because it’s more about a subtle (or not-so-subtle) sense that something feels wrong. Replacing basic logic keywords with a class should trigger that. But if you want a concrete problem, I’d say it’s about [easy-to-read code](https://web.archive.org/web/20071010144846/https://weblogs.java.net/blog/kgh/archive/2004/09/evolving_the_ja.html) versus “clever” code that’s harder to read. – VGR Dec 15 '17 at 18:35
  • 1
    @VGR: Harder to read for one navigation maybe. For two transforms you can't even use ternary without intermediate variables. This argument is basically the same as using for loops vs streams, and like that argument, when something is "simple enough" to use a procedural for loop is really only a question of style preference. Except in this case, ternary is already a workaround for lack of a null-safe operator, whereas a for-loop is distilled to the simplest form. – Mark Peters Dec 15 '17 at 19:24