Java 8 Optional-class: return value if nullable

Question

I'm trying to convert an existing snippet of code to some fancy Java 8 one-liner.

private static final Map<Integer, Foo> FOO_MAP = ...

public static Foo getForCode(final Integer code) {

  if (code == null) {
    return null;
  }

  final Foo foo = FOO_MAP.get(code);
  if (foo== null) {
    throw new IllegalStateException("Unknown foo for code: " + code);
  }

  return foo;
}

My solution so far, which is lacking the handling if the param is null.

public static Foo getForCode(final Integer code) {
  return Optional.ofNullable(code).map(FOO_MAP::get)
        .orElseThrow(() -> new IllegalStateException("Unknown foo for code: " + code));
}

Hopefully the “solution” illustrates why Optional is not a general replacement for if statements. Your original code is clearly superior. — VGR
– VGR, Commented Mar 5, 2018 at 15:07
Yep. I hoped that there was a real "one-line" replacement, which I was missing. — user871611
– user871611, Commented Mar 5, 2018 at 15:12
@VGR A single Optional instance is not designed to perform distinct checks as the OP wants but it doesn't mean that it should not be used as type of return of the method. Optional allows to convey that the return may contain nothing and it also prevents NullPointerException. — davidxxx
– davidxxx, Commented Mar 5, 2018 at 15:13
The behavior of returning null if the input is null, is questionable to begin with. The preferred behavior is “fail-fast” instead of letting the programmer trace backward through the program to find out, where the original null came from. When you replace Optional.ofNullable(code) with Optional.of(code) in your code, you already have the recommended behavior. — Holger
– Holger, Commented Mar 5, 2018 at 16:56

Roshane Perera · Accepted Answer · 2018-03-05 14:49:34Z

4

you can returning Optional<Foo> from getForCode(final Integer code) and let the client deal with the optional value returned.

public static Optional<Foo> getForCode(final Integer code) {
        return Optional.ofNullable(code).map(FOO_MAP::get);
}

answered Mar 5, 2018 at 14:49

Roshane Perera

1321 silver badge6 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Joop Eggen Over a year ago

Exactly: use the exception of get instead of the original one.

Stuart Marks · Accepted Answer · 2018-03-05 16:55:38Z

4

Without changing the behavior of the original function, I don't think Optional buys you anything here, except the opportunity to bury some complex conditionals within its methods and lambdas. If you were to minimize the conditional expressions, using if-statements and boolean expressions, you'd end up with this:

Foo getForCode(Integer code) {
    Foo foo = null;
    if (code == null || (foo = FOO_MAP.get(code)) != null) {
        return foo;
    } else {
        throw new IllegalStateException("Unknown foo for code: " + code);
    }
}

This is a tad more readable than any solution using Optional, to my eye, though it's still pretty obscure.

If you're willing to change the semantics of the function, and to return an Optional, then the suggestion from Roshane Perera seems like a reasonable approach (+1).

answered Mar 5, 2018 at 16:55

Stuart Marks

133k39 gold badges215 silver badges275 bronze badges

7 Comments

Holger Over a year ago

That does what the OP’s original code does, but I consider “return null if the input is null” as discouraged as overusing Optional (or even worse).

Stuart Marks Over a year ago

@Holger I agree; I detest the "return-null-if-input-is-null" antipattern. I had actually started to write an editorial comment on that very topic, but then it started to turn into an essay, which seemed like it would be a distraction from the answer. :-) But there seems to be a legitimate use case, which is that some object might or might not have a code, but if it has a code, it must be have an entry present in the map. I wouldn't express those semantics using "return-null-if-input-is-null" though.

davidxxx Over a year ago

@Stuart Marks Splitting a part of the logic in the client code (suggestion from Roshane Perera) means in a some way creating an artificial split and a reading indirection for a logic that should be at the same place. Besides, if the method is invoked multiple times you will have to repeat the duplication in the client code or worse introduce an helper method to do that. I am really not sure that it be fine.

Stuart Marks Over a year ago

@davidxxx It's hard to say without knowing more specifics about the OP's actual use case. But I'll note that the OP's code can return null, so the logic for handling that is already spread among all the callers. Returning Optional instead of a nullable reference makes this logic less error-prone, but it doesn't fundamentally change the callers' responsibility.

davidxxx Over a year ago

@Stuart Marks I agree about the broadness of the expected logic. " it doesn't fundamentally change the callers' responsibility." Of course but this solution changes the actual caller responsibility in the OP code without functional reason but having the getForCode() method lighter to handle the Optional. So, yes it is more elegant but maybe not the suitable way as handling a null from the client side is one thing but handling both that and the exception rising is another thing.

|

Eugene · Accepted Answer · 2018-03-05 14:53:34Z

2

It's not very readable, but you can :

Foo result = Optional.ofNullable(code)
            .map(x -> Optional.ofNullable(FOO_MAP.get(x))
                              .orElseThrow(() -> new IllegalStateException("Unknown foo for code: " + code)))
            .orElse(null);

answered Mar 5, 2018 at 14:53

Eugene

122k17 gold badges219 silver badges335 bronze badges

Comments

davidxxx · Accepted Answer · 2018-03-05 15:27:48Z

1

You could handle it alone first.
Trying mixing both cases in a single statement may make it less readable.

Note that actually you usage of Optional is probably not required.
Optional makes more sense for return type. In your actual code you go on to return null value.

Here is a usage of Optional where you return an Optional to handle both cases a returned Foo and no returned Foo:

public static Optional<Foo> getForCode(final Integer code) {
    if (code == null)
        return Optional.empty();

    Optional<Foo> optional = Optional.ofNullable(map.get(code));
    if (!optional.isPresent()) {
        throw new IllegalStateException("Unknown foo for code: " + code);
    }
    return optional;
}

edited Mar 5, 2018 at 15:27

answered Mar 5, 2018 at 14:44

davidxxx

132k23 gold badges231 silver badges228 bronze badges

4 Comments

Malte Hartwig Over a year ago

Sorry, I was to fast with editing and realised now: You are mixing return types. You return an Optional in case code is null, but else you return a Foo.

davidxxx Over a year ago

@Malte Hartwig No problem. I had just adapted the code of the question without looking carefully it. Don't worry, I updated with a correct code.

Malte Hartwig Over a year ago

What I mean is that orElseThrow returns Foo, but empty return Optional. You will have compiler errors one way or another when copying it into an IDE. You'd have to wrap the result of orElseThrow in an Optional again, for example, or check "manually" with isPresent.

davidxxx Over a year ago

@Malte Hartwig Thanks ! Not still used enough to Optional ! I will try with an IDE the last time :)

Collectives™ on Stack Overflow

Java 8 Optional-class: return value if nullable

4 Answers 4

1 Comment

7 Comments

Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

7 Comments

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related