Thursday, February 23, 2006

toString() ought to make life easier, BUT!

I have always liked a sensible toString() implementation that allows me to quickly grasp the important aspects of an object when I just have a look at it in the debugger or print it to some logger.

A few weeks ago I read Joshua Bloch's "Effective Java" which also encourages you to provide meaningful implementions of toString() in your programs to make life easier for yourself and others using your stuff.

Today I was asked if I could help a colleague of mine with an error that had occured in a production environment (have a look at Russ Olsen's Weblog to learn about the debugging guy...). He showed me a stacktrace like the following:

java.lang.NullPointerException
  at some.package.TheClass.toString(TheClass.java:2264)
  at java.lang.String.valueOf(String.java:2131)
  at java.lang.StringBuffer.append(StringBuffer.java:370)
  at some.other.package.InterestingClass.relevantMethod(InterestingClass.java:292)
  at some.other.package.CallingClass.aMethod(CallingClass.java:229)
  ...

In the really InterestingClass we have a try-catch block that catches exceptions and wraps them in more general ones and re-throws them, like this:

try {
...
} catch (SpecialCustomException e) {
   throw new MoreGeneralCustomException(this.getClass(), this, "Field access error: "+fieldName+" in "+anObject, e);
} 

However the following little piece of code managed to leave us totally in the dark about what had really happened:

public String toString() {
  StringBuffer tStringBuffer = new StringBuffer(256);
  tStringBuffer.append("ValueClass:");
  tStringBuffer.append(getAnAttribute().getSomethingElse());
  return tStringBuffer.toString();
}

As you may have guessed by now, the line with the getAnAttribute().getSomethingElse() is the "at some.package.TheClass.toString(TheClass.java:2264)" in the stacktrace above. Clearly someone did not think about the possibility of getAnAttribute() returning null. Because this NPE now occured while constructing the detail message for our own custom exception, it did never come to call the MoreGeneralCustomException constructor, which in turn would have told us where to look for the real mistake in the program.

I have looked through our application sources and found around 200 customized toString's. I guess I will have to go over them one by one to make sure something like that is not still lurking around somewhere else...

No comments: