Created
November 27, 2016 20:11
-
-
Save jbrains/2f4b038806edeb3638b637a1ac48ca5f to your computer and use it in GitHub Desktop.
I'm not convinced by this particular usage of Optional...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
public CommandResponse onBarcode(String barcode) { | |
Price price = catalog.findPrice(barcode); | |
return price == null | |
? new ProductNotFoundMessage(barcode) | |
: new ProductFoundMessage(price); | |
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
public CommandResponse onBarcode(String barcode) { | |
return catalog.findPrice(barcode) | |
.<CommandResponse> flatMap(price -> Optional.of(new ProductFoundMessage(price))) | |
.orElse(new ProductNotFoundMessage(barcode)); | |
} |
@nicolaiparlog Even better! Thanks. Yes: I might consider a factory method, although that would annoy me, since, in a sense, that's a big part of the point of this class. :)
If I didn't miss anything, it seems that catalog#findPrice
returns Optional
?
I prefer to not have Optional
as a return type (nor as a parameter for that matter) for (at least) these reasons:
- have to change or wrap existing APIs
- the
Optional
instance can also benull
(the Java language enables it ;)) so not really solving the problem
I prefer to use Optional.ofNullable
.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Thanks, @tumbarumba. Almost. On Java 8 update 112 I have to explicitly specify the type
CommandResponse
, otherwise Java infers the wrong type for the "or else" branch.I didn't understand this when I first tried to write the code, so that might explain how I ended up going down the
flatMap()
road.