Don't Code Like This!

WARNING: This blog entry was imported from my old blog on blogs.sun.com (which used different blogging software), so formatting and links may not be correct.

I was trying an open source web framework yesterday, and couldn't get it to work so I stepped through some of the code on the server with a debugger, and came across some code fragments that I thought I would submit to the Hall of Shame.

Besides, it's fun to blog about this, because each time I mention coding style I invariably get passionate responses from people defending their beloved style.

In the following, the class and method names have been changed to protect the guilty.

At first, I simply got a generic JspException. The code was doing this:

...
} catch (ConfigNotFoundException ex) {
throw new JspException(msg);
}

The original stack trace is not chained to the new exception! Please pass it in - JspException
has a constructor which takes both a message and the root cause exception, ex in the above.
If you're using some Exception class which is older and doesn't let you pass in the root cause,
such as DOMException,
then first create the exception, then call the
initCause
method on it, and finally throw the exception object.

The second code fragment I came across was this:

try {
return findConfig(servletContext).getResult(foo, servletContext);
} catch (NullPointerException ex) {
throw new ConfigNotFoundException("Can't find configuration");
}

The above code is using exceptions for flow control. Rather than first
calling findConfig, then checking if it is null, it just goes
ahead and tries calling it anyway, relying on the NullPointerException to
handle this case.

The code also has the bad effect of catching all unintended null pointer
exceptions in the subsequent getResult method call. These are
unrelated to the ability to find the configuration, so the error message would
be misleading. (And again the original exception is not chained).