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<String> 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.
| Reference: | When a String is Null but Not Null from our JCG partner Daryl Mathison at the Daryl Mathison’s Java Blog blog. |
Looks to me as if you’re just dealing with the effects of the bug not with the real cause.
Do you have access at the point where the variable is set to “null” instead of null?
If you have access, then the real solution is to fix the assignment to the variable, not create a hack in a validate method.
Sometimes the cause is some one else’s buggy code and does not have any access to the cause. I have seen many times where the situation is not my fault, still my problem.
My two cents are regarding your “StringValidator” class.
I think you’ll benefit to hold your “dirtyWords” in a Set and later to query if the Set contains the test string rather than iterate over the hole List of “dirtyWords”.
Otherwise, its an interesting bug and a good article.
agreed about changing the List to a Set. One of the great things and bad things about software development is that code is never “done.”
My response to this is that this is not a big. It is by design and the original code is correct. The file parser has the bad code and must be corrected. This falls under the category of input validation. What if the field should contain the value “null”? This hack won’t ever store the value. And if the rule is to parse the text field value of “null” to be a null value (not set) that would be one if the data processing rules for the file parser, not the business logic. In my experience, I find it makes… Read more »
There is a big assumption here that the input came from a file. What are you referring to as a hack. I have three examples of validation that can be hacky. A rules engine would be better but assumes the input comes from a file which was never mentioned. If it was a file, you are right, the parser needs to pick up the slack. But consider that one of the jobs of an object is to protect itself from bad inputs. The bug discovered here was a definite bad input.
See my post that I added to the bottom of the thread. I directly address your “bug” claim in the String class, and attempt to explain the underlying issue you feel is a bug. I believe we all understand the java String issue. As such I think we almost all were addressing the actual issue, not picking up on the fact you believed a String can hold a value of null, when in fact, no variable (object or primitive) can ever have a value of null — only the assignment of a value or the unassigned state (defined in java… Read more »
The null value is assigned to the String variable only when toString is invoked on it.Checkout toString() of String class
That bug is probably String class’ fault… the String.valueOf() static method returns the string “null” when given a null reference as a parameter. Why the JDK guys decided that returning “null”, the String, made more sense than just returning null, the reference, is beyond me.
What is funny is that of all the posts I have made where more mistakes can be made and rightfully so get a tongue lashing over it, this one witch really is just a warning to other developers(this post) gets the most heat about how I did the code and not about the bug itself.
Your assertion is that String.valueOf(null) returning “null” instead of null is a bug. However, that is not the case. It may be undesirable behavior to you however, a properly formed valueOf() function never returns null as the value, instead it throws an exceptions. valueOf should always return an instance of the Object, and null is the unassigned state of the variable being assigned, not the value of the object. The source of String.valueOf(Object) is very clear that the observed behavior is by design: /** * Returns the string representation of the Object argument. * * @param obj an Object. *… Read more »