12

A super simple question:

Here's my plain Java code using traditional ternary operator ?

public DateTime getCreatedAt() {
    return !recordA.isPresent() ? recordB.get().getCreatedAt() : recordA.get().getCreatedAt();
}

My best bet is following:

public DateTime getCreatedAt() {
    return recordA.map(
        record -> record.getCreatedAt())
        .orElse(recordB.get().getCreatedAt());
  }

This could compile, but looks like it's not behaving correctly. It always executes both branches, for e.g. when recordA isPresent(), it still executes recordB.get().getCreatedAt() which throws me

java.util.NoSuchElementException: No value present

Any help is appreciated!

Basically, I'd like to replace the traditional ternary operator with more advanced Optional/lamda features.

Undo
  • 25,519
  • 37
  • 106
  • 129
Fisher Coder
  • 3,278
  • 12
  • 49
  • 84
  • 7
    `.orElseGet(() -> recordB.get().getCreatedAt());` ? – Ousmane D. Nov 08 '18 at 22:47
  • 4
    You need `orElseGet`. You can also use two optionals and `or()`. – Boris the Spider Nov 08 '18 at 22:48
  • 1
    Is `recordB` Optional too? – tsolakp Nov 08 '18 at 22:49
  • @BoristheSpider `or()` is only available since Java 9. – shmosel Nov 08 '18 at 23:01
  • Yes, `recordB` is Optional as well, sorry just saw it. @tsolakp – Fisher Coder Nov 08 '18 at 23:18
  • 1
    See my answer on how to handle if it is empty. – tsolakp Nov 08 '18 at 23:19
  • Awesome! Thanks! @tsolakp – Fisher Coder Nov 09 '18 at 01:09
  • 2
    `Optional` is not meant to be used this way. I mean, you can use it like this if you want, it's your code after all :) But my point is that it wasn't designed to replace the ternary operator or if/else statements. Its main usage is as a return type, seach in youtube Stuart Marks' conference about this topic. You can improve your actual code like this: `return (recordA.isPresent() ? recordA : recordB).get().getCreatedAt()` – fps Nov 09 '18 at 01:51
  • Most of all I'm puzzled at what problem is being solved? The original code using a ternary is IMHO the best readable of all proposed solutions. I could understand if people were trying to move from imperative to more functional programming, but a ternary expression is already an expression, so it's hard to get "more functional". So what is your definition of "advanced"? – Stephan Herrmann Nov 17 '18 at 00:47
  • @StephanHerrmann I understand the original ternary solution is concise enough, I just wanted to take full advantage of the new lamda function in Java. Thanks – Fisher Coder Nov 19 '18 at 11:03
  • @FisherCoder for an exercise nothing wrong with trying different styles. In terms of "taking advantage" I simply don't see the advantage gained in this case :) – Stephan Herrmann Nov 20 '18 at 00:43

4 Answers4

9

To avoid eagerly evaluating else-branches, use orElseGet, which takes an instance of the functional interface Supplier:

return recordA.map(
    record -> record.getCreatedAt())
    .orElseGet(() -> recordB.get().getCreatedAt());
nanofarad
  • 40,330
  • 4
  • 86
  • 117
  • 5
    If `recordA` and `recordB` are of the same type, it can be shortened to `recordA.orElseGet(recordB::get).getCreatedAt()` – Andreas Nov 08 '18 at 23:05
  • @Andreas that does not work when `recordB` is empty `Optional`. – tsolakp Nov 08 '18 at 23:26
  • 1
    @tsolakp It works exactly the same as the question and this answer, i.e. all 3 will throw exception if both `recordA` and `recordB` are empty. – Andreas Nov 08 '18 at 23:29
  • @Andreas. Not sure if OP wants to get exception if `recordA` and `recordB` are empty. – tsolakp Nov 08 '18 at 23:31
  • 2
    @tsolakp But why are you addressing my comment, as if my commented code is the only one with the problem? Address the answer too, because it has the same problem. Better yet, write a comment to the question, informing OP of potential problem, and ask if it is an issue. Perhaps logic is that at least one of them will be non-empty, making it a non-issue. It is certainly not a problem introduced by my suggested shorter code, like it would have been if I wrote `orElse(recordB.get())`, but that's not what I wrote. – Andreas Nov 08 '18 at 23:35
  • @Andreas. Sorry, did not want to pick on you:) I did like your shorter version, just wanted to point to the issue it has. BTW, I did address it with OP. – tsolakp Nov 08 '18 at 23:39
5

You're looking for .orElseGet(() -> recordB.get().getCreatedAt()); and the reason to that can be found on this post --> Difference between Optional.orElse() and Optional.orElseGet()

Some people may find it a bit subjective but personally, I'd say by default, it makes more sense to use orElseGet() as opposed to orElse every time except in the case where the default object is already constructed as this will prevent many unexpected problems (given you didn't read the differences between orElse and orElseGet doc) as such of the one you're facing now.

read more from Java Optional – orElse() vs orElseGet()

Ousmane D.
  • 54,915
  • 8
  • 91
  • 126
5

My question about recordB being Optional got unanswered but if it is Optional then you cannot just safely call its get method, you need to check if it is empty or not. Here safe call to get record or null if both recordA and recordB are empty Otionals.

        recordA
            .map(Record::getCreatedAt)
            .orElseGet( () -> recordB.map(Record::getCreatedAt).orElse(null) );
tsolakp
  • 5,858
  • 1
  • 22
  • 28
  • 2
    I don't see the purpose of the `Optional` in the second part. Creating a `Optional` just to call `orElse(null)` on it is a known antipattern. – Boris the Spider Nov 09 '18 at 07:36
  • I agree. But we are working within example provided by OP. Ideally `getCreatedAt` should return `Optional` too or least use some type of default value for `DateTime` instead of `null` in my answer. – tsolakp Nov 09 '18 at 15:31
0

If you are using Java 9+, you may be able to use ifPresentOrElse() method as more completely explained in this Stackoverflow answer.

recordA.ifPresentOrElse(
   value -> recordA.get().getCreatedAt(),
   () -> recordB.get().getCreatedAt()
);
Brad Parks
  • 66,836
  • 64
  • 257
  • 336