Introduction

A junior programmer on my team at work had an interesting bug.  Its symptoms I have seen more than once.  This post is what to look for and how to prevent it in the future.  I also explore different solutions to the problem.

Symptoms

The code in question looked well made.:

if(trouble != null && !trouble.isEmpty()) {
    System.out.println("fine here: " + trouble);
} else {
    System.out.println("not so fine here: " + trouble);
}

The code would hit the “fine here” block but would print the value of “null.”  The variable was set by reading a file.

Investigation

The developer and I looked at the print out and ran the test several times but the same result came up.  I looked where the variable was being set.  It should have set the value to null if there was nothing there, yet the print out stated the value was null.  I had an idea and we decided to test it.  He changed the code:

if(trouble != null && !trouble.isEmpty() && !trouble.equals("null")) {
    System.out.println("fine here");
} else {
    System.out.println("not so fine here");
}

The tests went to the “not so fine here” every time.  It appears the value was set to the string “null” not to the value null.

What to Learn

To tell the truth, I have seen this before.  It took me about a day when my code started doing the same thing to figure it out.  What I learned from this is that parameter checking is still a good thing.  I found that the valid string check could be used in several places in my code.  To prevent the copy and paste anti-pattern, I abstracted the validation into a method.

private static boolean isValidString(String test) {
    boolean isValid = (test != null && !test.isEmpty() && !test.equals("null"));
    return isValid;
}

The next step to prevent a longer and longer and validation line is to abstract it to a validation object.  This allows for a dirty word list.

public class StringValidator {
    private List dirtyWords;

    public static final int ARRAY_SIZE = 20;
    public StringValidator() {
        dirtyWords = new ArrayList(ARRAY_SIZE);
    }

    public boolean isValid(String test) {
        boolean isValid = false;
        isValid = (test != null) && !test.isEmpty();
        if(isValid) {
            for(String word: dirtyWords) {
                if(word.equals(test)) {
                    isValid = false;
                    break;
                }
            }
        }
        return isValid;
    }

    public void addDirtyWord(String word) {
        if(!isValidString(word)){
            throw new IllegalArgumentException(word + " is not a good dirty word");
        }

        dirtyWords.add(word);
    }

    private boolean isValidString(String test) {
        return ((test != null) && !test.isEmpty());
    }
}

which leads to this parameter checking code:

if(validator.isValid(trouble)) {
    System.out.println("fine here");
} else {
    System.out.println("not so fine here");
}

Conclusion

Sometimes we need to think a little outside the box to figure out a problem.  Don’t feel bad to get a second set of eyes on a problem; it maybe the best thing that happened.  I explored the solution ending up creating a validator allowing for the inclusion of a dirty word list without a long and confusing test. The source code for this can be found at https://github.com/darylmathison/null-but-not-null-example.