I was disappointed by the introduction of Optional to the Java language, because I've been using @Nullable and @NotNull annotations for years, and it's a much simpler solution. The big advantage of this approach is that it makes null pointers a compile-time error rather than a run-time bug. Now that I've worked with Optional and seen it in action in production code, I've only strengthened my earlier view. Here are some examples, all taken from production code, of bad usages of Optional, with notes on what it would look like with the NullnessChecker annotation processor instead.
01 private void someMethod(Widget widget, ... <other parameters>) {
02 Optional<Widget> widgetOpt = Optional.of(widget);
03 if (!widgetOpt.isPresent) {
04 throw new BusinessException("Missing Widget");
05 }
06 // ... widgetOpt is never used again
07 // ... several lines later
08 Optional<Widget> widgetMaybe = Optional.of(widget); // this duplicates the previous Optional!
09 processData(widgetMaybe);
10 // ...more code
11 }
What's going on here? On lines 2 and 3, the coder is taking two lines to do a simple null check. Then widgetOpt
is thrown away, and a second Optional of the same value is created on line 8, because they call a method that requires it.
The first use is needlessly verbose, and seems to be done just to avoid ever using the null
keyword in the code, as if bansishing it will prevent null-pointer bugs. (It won't.) Lines 02 and 03 could have been made cleaner, more readable, and even marginally faster by writing if (widget == null) {
The second use of Optional is necessary only because the API they're using requires it.
To make matters worse, there's a bug in line 02 that didn't get caught. It says Optional.of(widget)
, but it should say Optional.ofNullable(widget)
! So if a null value ever gets passed into this method, it will throw a NullPointerException
instead of the BusinessException
thrown on line 04. (The second use of Optional.of()
in line 08 is fine, because we already know it's not null.) But nobody caught this error for two reasons. First, There was no unit test. Second, The code that called this private method had already made sure that widget was not null, so the test was unnecessary.
01 private void someMethod(Widget widget, ... <other parameters>) {
02 // ... several lines later
03 processData(widget);
04 // ...more code
05 }
The annotation processor defaults to @NonNull, so if I added a null-check, it would tell me that it's unnecessary. The annotation processor knows that the calling method already checks for null, so it's happy with this. If somebody later removes the test for null in the calling method, it will generate a compile-time error where this method is called.
Here's a class that was generated by Swagger 2.0, using their Swing server generator, with the Java 1.8 option turned on:
@Controller
public class MenuItemApiController implements MenuItemApi {
private final ObjectMapper objectMapper;
private final HttpServletRequest request;
@Autowired
public MenuItemApiController(ObjectMapper objectMapper, HttpServletRequest request) {
this.objectMapper = objectMapper;
this.request = request;
}
@Override
public Optional<ObjectMapper> getObjectMapper() { return Optional.ofNullable(objectMapper); }
@Override
public Optional<HttpServletRequest> getRequest() { return Optional.ofNullable(request); }
}
This has two private final members, and two getters that wrap the values in an Optional. But take a look at that constructor. It's annotated with @Autowired. Spring will automatically instantiate valid values for both of the parameters and inject them into the constructor. And they're final, so they'll never change to null. The methods are returning objects that can never be null, wrapped inside an Optional. Does the use of Optional here add clarity to the API? It actually does the opposite: It suggests that two never-null objects might actually be null, forcing users to write extra code to check for null values that they'll never see.
@Controller
public class MenuItemApiController implements MenuItemApi {
private final ObjectMapper objectMapper;
private final HttpServletRequest request;
@Autowired
public MenuItemApiController(ObjectMapper objectMapper, HttpServletRequest request) {
this.objectMapper = objectMapper;
this.request = request;
}
@Override
public ObjectMapper getObjectMapper() { return objectMapper; }
@Override
public HttpServletRequest getRequest() { return request; }
}
Again, this is simpler, because @NonNull is assumed. The constructor can't get called with null values, so we don't bother with Optional.
Many people have written guidelines recommending against using Optional as parameter methods, but people do it anyway. A common variation of the code in example 1 looks like this:
01 private void someMethod(Optional<Widget> widgetOpt) {
02 if (!widgetOpt.isPresent) {
03 throw new BusinessException("Missing Widget");
04 }
05 // ... (More code)
Again I have to ask: What clarity is being added to the API by using Optional? To anyone looking at the API, it implies that null is a valid input value. But once you look at the code, it's clearly not. Putting Optional on a parameter that shouldn't be null results in a misleading API that's also more verbose to call, since the user must now write someMethod(Optional.ofNullable(widget));
instead of someMethod(widget);
A good interface should take care of boilerplate details needed to perform its function. By using Optional in a parameter, this forces the user to add a bit of boilerplate that doesn't add any value.
01 private void someMethod(Widget widget) {
02 // ... (More code)
Again, @NonNull is assumed, so we don't need to check for null. A value that may be null will generate a compiler error.
Here's a case where putting Optional on a method parameter doesn't mislead the user:
01 private void someMethod(Optional<Widget> widgetOpt) {
02 final Widget widget = widgetOpt.orElse(getDefaultWidget());
03 // ... (More code)
Here, finally, we can say that Optional is adding clarity to the API. Use of null instead of a Widget instance is actually allowed. Here, the API does not mislead anyone. Of course, users who choose to use null must be careful not to call Optional.of(widget)
. Instead, they should use Optional.ofNullable(widget)
or Optional.empty()
, but that's a fail-fast mistake, so it will probably get caught early. Unfortunately, so many developers wrap mandatory parameters inside Optional, that the meaning of this occasional valid use will often get lost anyway. And, there's a simpler way to write the API that doesn't add verbosity to the calling method:
01 private void someMethod(final Widget widgetOrNull) {
02 final Widget widget = widgetOrNull == null? getDefaultWidget() : widgetOrNull;
03 // ... (More code)
Simply renaming the parameter will provide the same information as Optional. Before Optional was invented, not many people did this, which is probably a shame, because it adds clarity without adding verbosity. It also eliminates the (minor) bug that will arise if the calling method uses Optional.of()
instead of Optional.ofNullabe()
.
01 private void someMethod(@Nullable widget) {
02 widget = (widget == null) ? getDefaultValue() : widget;
03 // ... (More code)
The @Nullable annotation becomes part of the API, so users can see that a null value is allowed. The Nullness checker will figure out that widget
is not null, and proceed from that assumption.
01 private void someMethod(Optional<Widget> widgetOpt) {
02 if (!widgetOpt.isPresent) {
03 throw new NullPointerException();
04 }
05 Widget widget = widgetOpt.get();
06 // ... (More code)
Yeah. I've seen this. Just get rid of lines 2 through 4. It does exactly the same thing.
01 private void someMethod(Widget widget) {
02 // ... (More code)
Once again, passing a possibly-null value will generate a compile-time error.
01 public User findByUuid(@NotNull String userUuid) {
02 UserTable userTable = (UserTable)this.userTableDao.findByUuid(userUuid).orElse(null);
03 return userTable != null ? this.userServiceConverter.convertFromUserTable(userTable) : null;
04 }
Huh? (More on this later.)
I'll be honest, this is a problem with java util Optional, but it's not a problem with the concept of using an Optional type in general. Most of your complaints exist because there's no peekEmpty method equivalent on the java util Optional class. The wrong answer to this is to throw away allowing the compiler to understand nullability by removing all optionals. The right answer is to use libraries that did make a "fully usable" option/maybe class in java: like: vavr option: https://www.vavr.io/vavr-docs/#_option or cyclops option: https://www.javadoc.io/doc/com.oath.cyclops/cyclops/10.3.0/cyclops/control/Option.html. Look at those juicy convenience methods on the class. Remember, there's literally nothing special about the java util Optional class besides being in the sdk, and the compiler help comes from the fact that you can't pass an
Optional<String>
into a method that expects aString
You keep referencing this, but in many functional languages, the general purpose Maybe type is just a library class with no special syntax. Look at the option class available in scala: https://www.scala-lang.org/api/current/scala/Option.html or (if you want to go fancy) datatype in haskell: https://hackage.haskell.org/package/base-4.14.0.0/docs/Data-Maybe.html. The random nullable type examples people give (like kotlin) with
.? or ?:
are just a very special instance of the flatmap method, and it somewhat bothers me when people think making syntax for a single case (nullability) instead of the general case (arbitrary flatmap) is somehow "special" (and it wouldn't even resolve your issues!). I'm actually really confused as to who the target people are that would wantOptional<String>
andString
to work together, so I'll be honest, I'm lost as to what Brian means by language support for it (and unfortunately, he hasn't expanded on it in the answer).As an example of what I mean, you can use the same ideas for nullability for exception chaining as well. Imagine two methods like
invoked like
Now imagine two methods like
and invoke it like
Similar code (there's a common pattern), but one handles methods returning null and one handles methods "throwing" an exception. You want another example? Replace Try/Option with Future and now you have async methods, and the best part? It's all composable. For try, you also get something way better than checked or unchecked exceptions, and the consumers of the code are forced to handle the exception (with an easy way to throw it up if they don't want to since it is java still). Now the compiler helps you deal with exceptions (using Try) or null (using Option), or even both with a signature like
, and the method signatures convey what's happening and the compiler forces you to handle some error case and the case that the file might be empty because you can't pass a
Try<Option<File, Exception>>
to a method that expects aFile
. The reason I show this is that there's basically nothing special about the java util version of optional except it's neutered in terms of the methods that exist on it, but you can fix that and you didn't need introduce a single.?
or?:
to do it.As to the specific examples (which seem like they got changed since I responded)
Looking at
Without making other assumptions, I'd agree with just doing a null check and throwing and changing processData to take a
Widget
instead of anOptional<Widget>
(because taking inOptional<Widget>
as a parameter in any method is almost always wrong). If you really want to dig into it though, I'd rather the calling code bomb out before even invoking this method in the first place because I'm not a big fan of throwing business exceptions from validations in methods really far down the call stack. Is it actually the case thatsomemethod
should sometimes be called with null widgets and other times be called with actual widgets or could the places where widget might be null above it throw the exception instead and remove the null check entirely from this method? Like is this method validating data and doing work of some sort? If so, it feels like it's doing too much and you can separate validating (say user input) from the work that needs done if the input is good. Is it just a super defensive null check? If so, maybe just let it NPE?Look at the current example 4. That case is just handled with method overloading (literally an example that Brian gives for not using method parameters) with
and client code can invoke it with
or if you don't want to expose default widget
No nulls necessary at all.