CodeSOD: Optional Ternaround

As we frequently discuss here, many versions ago, Java added functional programming expressions, which can allow a developer to write very expressive, simple code. Of course, we don't cover any of those uses, because that's all fine.

Kevin L used to work for a Fortune 500 company, and that company was full of developers who got very excited about those functional programming features in Java. Very excited. Too excited, if we're being honest.

For example, if they wanted to check if an object was null, they might do this:

Optional.ofNullable(someObject) .orElse(null)

This code uses the Optional type to wrap a potentially nullable object, someObject, in an option, and then return the value of the object orElse return null. So this line takes an object which may be null, and if it isn't null, returns the value, otherwise it returns null.

Or, they might do something like:

private boolean detectChanges(String checksum, String checksumFromDB) { return Optional.ofNullable(checksumFromDB) .map(defaultsChecksumDB -> !defaultsChecksumDB.equals(checksum)) .orElse(Boolean.TRUE);}

This function takes the potentially nullable value checksumFromDB and attempts to map it using a lambda function, which simply checks if it's not equal to checksum. If it's null, this doesn't work, so they helpfully orElse return Boolean.TRUE. Assuming that checksum isn't null, this could be a very simple !checksum.equals(checksumFromDB), but instead they chose this.

But the best example of the lengths that these developers would go to to avoid using a conditional, 'in the name of readability', would be this one:

boolean isEmptyOrNullOrStringNull = StringUtils.isBlank(taxCode);//The point here is to treat the blank string as an actual NULL,this.taxCode = Optional.ofNullable(taxCode) .map(i -> isEmptyOrNullOrStringNull ? null : taxCode) .orElse(null);

The goal of this is to treat an empty string as a null value. So first, they check if it's empty or null using isBlank. Then, they wrap it in an optional, map the optional to a null if it isEmptyOrBlank, and if the value returned from the map is null, we return... null.

A simpler way to write this, and the one Kevin submitted as a pull request, was this:

this.taxCode = StringUtils.isBlank(taxCode) ? null : taxCode;

There were protests from the functional programming camp on the team. Ternaries, of course, are "not readable", "they're confusing", they're "playing code golf", or "The Daily WTF has written so many articles about ternaries being bad!". The rest of the team, on the other hand, much preferred the simple, one line version that didn't spawn unnecessary types or lambdas in the process, so Kevin was able to get enough approvals to merge his version into the code base.

Kevin doesn't work there any more, but I can only assume the endless battle between ternaries and optional abuse continues to this day.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!