On pointless Java 8 embellishments, or an exercise in simplicity

While performing a code review, I stumbled upon this little gem:

Long num = someFunctionThatNeverReturnsNull();
if(Optional.ofNullable(num).orElse(0l) > 0) {
  // ...
}

The salient part, of course, is the if-statement. It’s rare to come across a single line with so many layers of wrongness.

  1. First, let’s talk about 0l. Depending on your font, that might look like zero one, zero el, o one or o el. By convention in Java, this should be zero EL: 0L. This is easier to read, and the suffix L makes it clear that we’re dealing with a long instead of an integer. That would look something like this:
    if(Optional.ofNullable(num).orElse(0L) > 0) {
      // ...
    }
  2. Second, what’s going on with these data types? 0L is now obviously a primitive long which will be auto-boxed to a Long object. num is a Long object. And then there’s the dangling 0. Which is a primitive integer (int). For one reason or another. Now, I admit that casting to long or int is pretty cheap. This is never going to be a performance issue. But consistency is a good thing. You probably want to compare like with like.
    There’s also a bit of weird auto-(un)boxing going on here. 0L will be auto-boxed to Long. But the result of the orElse() bit will be unboxed to a primitive long. The comparison will then be comparing a long to an int, which causes the int to be widened to a long.
    There isn’t much we can do about the auto-(un)boxing in this case, considering the comparison we want to perform. But we can at least ensure we’re using consistent data type width. So this:

    if(Optional.ofNullable(num).orElse(0L) > 0L) {
      // ...
    }
  3. And lastly, why the fuck are they using Optional in the first place? Never mind the fact that num can’t even be null here. The “plain”, non-embellished form of this statement would have been shorter and easier to read. That would look something like this:
  4. if(null != num && num > 0L) {
      // ...
    }

Here is the “corrected” version:

long num = someFunctionThatNeverReturnsNull();
if(num > 0L) {
  // ...
}

I can’t fathom why anyone would write garbage like this. It’s pointless. It’s hard to read. It’s ugly. And it’s bloody inefficient.

Leave a Reply

Your email address will not be published. Required fields are marked *