Summary
Tempted to code defensively? Maybe it's because you're dealing with offensive code.

---

Advertisement

Here's a little sample of code:

private void addResolution(List<Flag> flags) {
    if (flags != null) {
        if (supportedAdapters.contains(Adapter.LOW) 
                || supportedAdapters.contains(Adapter.MEDIUM)) {
            flags.add(new ResolutionFlag(ADAPTED));
        } 
    }
} 

What’s wrong with it?

No, it’s not the control flow. Sure, we could replace the conditional with a guard clause and get rid of some indentation, but the problem is more subtle than that. Most people are blind to it because it’s in their face every day.

The problem is this line:

if (flags != null)

Spurious null checks are a symptom of bad code.

That’s not to say that null checks are wrong. If a vendor gives you a library that can return null, you’re obliged to check for null. And, if people are passing null all over the place in your code, it makes sense to keep putting some null checks in, but, you know what? That just means that you’re dealing with bad code: you're dealing with code where people are actively making extra work for themselves and making code brittle in the process.

I’ve gotten more diplomatic over the years, but in one my early programming jobs, the guy who hired me called me over to his office and showed me a scheme that he’d created for our classes. Every method that we called could set a flag if it determined that it was passed a bad parameter. After a client called a method, it could check the flag and determine whether the call was successful or not. I said “Oh, cool, we can call it the ‘was I stupid flag!’” He gave me a withering look I’ll never forget, but he saw the point despite my youthful obnoxiousness: it was kind of silly to check afterwards if you could check before. Why check for an error afterwards if you can avoid it to begin with?

Passing null in production code is a very bad idea. It’s an example of what I call offensive programming – programming in way that encourages defensive programming. The better way to code is to deal with nulls as soon as they happen and translate them to null objects or exceptions. Unfortunately, this isn’t common knowledge.

Let’s look at an example.

Here’s a method that builds a set of parameters from an array of integers:

public String [] getParameters() {
    if (intParms.length  == 0) {
        return null;
    }
    String [] result = new String[intParms.length];
    for(int n = 0; n < intParms.length; n++) {
        result[n] = "parameter " + intParms[n];
    }
    return result;
}

Will everyone remember to check the return value for null? Probably not. Okay, let’s clean it up:

public String [] getParameters() {
    String [] result = new String[intParms.length];
    for(int n = 0; n < intParms.length; n++) {
        result[n] = "parameter " + intParms[n];
    }
    return result;
}

If we don’t have any parameters, we return an empty array. The empty array is an example of a null object.. an object which gracefully handles an exceptional case. Now that we're always returning a valid array, there's one less possible error.

When we program we always have the choice to program in a way which forces us to check for errors later or not. The alternative is finesse the problem, to design in a way that makes errors impossible. Making errors impossible is far better than handling them.

Let's revisit the original example:

private void addResolution(List<Flag> flags) {
    if (flags != null) {
        if (supportedAdapters.contains(Adapter.LOW) 
                || supportedAdapters.contains(Adapter.MEDIUM)) {
            flags.add(new ResolutionFlag(ADAPTED));
        } 
    }
} 

The method is private. If we go back to all of the callers we can change them so that flags can never be null. Then, we can clean up the code:

private void addResolution(List<Flag> flags) {
    if (supportedAdapters.contains(Adapter.LOW) 
            || supportedAdapters.contains(Adapter.MEDIUM)) {
        flags.add(new ResolutionFlag(ADAPTED));
    } 
} 

What we've done here is introduce a stupid-free zone in the code. If we tend it, it can grow.

Talk Back!

Have an opinion? Readers have already posted 67 comments about this weblog entry. Why not add yours?

RSS Feed

If you'd like to be notified whenever Michael Feathers adds a new entry to his weblog, subscribe to his RSS feed.

Digg | del.icio.us | Reddit

About the Blogger

Michael has been active in the XP community for the past five years, balancing his time between working with, training, and coaching various teams around the world. Prior to joining Object Mentor, Michael designed a proprietary programming language and wrote a compiler for it, he also designed a large multi-platform class library and a framework for instrumentation control. When he isn't engaged with a team, he spends most of this time investigating ways of altering design over time in codebases.