I was completing a code review for a recent merge/pull request and one of the questions that arose from this was the use of Optional.ofNullable() for checking if something was null and assigning a value if not. There was quite a bit of back and forth because really, the Optional route looks really nice and clean and is very easy to understand. Eventually, we agreed that we should be using a Ternary Operator in these instances, I’ll explain why we came to that decision below.
Optional.ofNullable
The initial code contained something like the following:
final String apiKey = Optional.ofNullable(config.getApiKey()).orElse("defaultValue");
This returned the desired outcome, if config or the API key is null, it will return the String “defaultValue”. There’s nothing really that wrong with the above, it’s pretty readable in the grand scheme of things, but the problem arises when you think of firstly, the performance impact and secondly the original design behind what Optional was created for.
The performance impact could possibly be more significant than a simple ternary operator or if/else statements, depending on how “big” these commands get in terms of what they are extracting.
The main point however is that this isn’t really what Optional was designed for. They were added to Java to try and remove the possibility of getting NullPointerExceptions by forcing the developer that is using an API to acknowledge that whatever they are being returned could be null. Not for basic null checking in your main code.
Ternary Operator
Anyone that has been a developer for any amount of time will usually understand what a ternary operator is. It’s basically a shorthand if/else statement. If we use the same example as above, it will look something like this:
final String apiKey = config.getApiKey() == null ? "defaultValue" : config.getApiKey();
This is simple, easy to read and doesn’t take up any extra space in the editor, making it a really nice option to use. It also comes without the potential performance impact that the Optional route might throw up due to its misuse.
If/Else
Now we all know what an if/else statement looks like. It’s the bread and butter of any sort of logic in an application. Translating the above example to its if/else form would look like:
String apiKey = null;
if(config.getApiKey() == null) {
apiKey = config.getApiKey();
} else {
apiKey = "defaultValue";
}
This takes up much more room than both examples above, so I can see why the initial developer for this merge request wanted to use the Optional.ofNullable() route, however, we can clearly see that if you have too much of this sort of logic in your codebase, it’s going to get very messy, very quickly.
Summary
Oh, the joys of programming. The problem we often run into when doing a code review is that personal preference plays a massive part and unless you have a set of predefined rules or practices that your team will follow, this sort of situation is bound to come up every now and again. This situation is slightly different in the sense that Optional wasn’t the right move regardless of personal preference, however, it worked and it looks good so we can see why the developer went this route. Fortunately, we have a very open team that is more than happy to take feedback on anything that comes up in code reviews and have a sensible discussion, something we should all strive to do!