CodeSOD: Did You Null This?

If I were to catalog my biggest failings as a developer, it's a carelessness around defensive programming. It's tedious, and it takes work and forethought, and honestly, I just want to get the thing working and see the results. But I recognize it's important, and work on developing that mindset.

Today's anonymous submitter found some Java code that is extremely defensive.

public String bundle() {// snip if (ISO8583.this == null) return null;// snip}

This code is deep down in a message parsing/constructing module, responsible for exchanging ISO 8583 messages- transactions for interacting with a payment card provider.

Now, this is the only return null in this method, and the calling code never actually checks for a null. So it's probably pretty fortunate that this could never actually be null.

Well, "could" is perhaps too strong. It should never be null, but you'll note here we're using a "qualified" this. This means there's a Java outer class involved, something like:

public class ISO8583 { public class MyInnerClass { public String bundle() { //snip if (ISO8583.this == null) { return null; } //snip } }}

ISO8583.this refers to an instance of the outer class, which should have a value, but if you're doing fancy reflection things, it might be null. You can reach inside the class and explicitly set it to null, if you wanted to for some reason. But you shouldn't be doing fancy reflection things, right? Certainly not to muck with built-in behaviors, right? It performs badly, it's complicated, and is usually YAGNI territory or premature abstractulation.

Our anonymous submitter adds:

My only hope is that this was a result of the fact that before joining this team, a previous developer got fancy into reflection and some how accessed this function from a non-instantiated object, but the reflection code is a WTF for another time.

Of course the previous developer was doing something "clever". The real WTF is that the null check may have actually been necessary, even if it shouldn't have been.

The entire ISO 8583 message layer has since been rewritten, but our submitter remains haunted by the implications of this block.

[Advertisement] Ensure your software is built only once and then deployed consistently across environments, by packaging your applications and components. Learn how today!