Java Exception Anti-Patterns 101
What could possibly go wrong ?
As a Senior Java Developer, I get to review a lot of code from beginning developers, with highly variable quality.
One of the most frequent sources of bugs that I see is incorrect handling of error conditions. Most programmers aren’t inclined to think about all the possible things that could go wrong with their program and the best way to handle each of these errors.
The most common method for handling errors in Java is exceptions, the topic of this article.
Recently I saw some Java code like this:
try {
…
} catch (Exception e) {
throw new MyException(“something went wrong”);
}
what’s wrong with this code ?
If something goes wrong and we have to debug it, the exception we caught may contain valuable clues as to what went wrong, which we have just thrown away, making debugging harder.
How about this version ?
try {
…
} catch (Exception e) {
throw new MyException(“something went wrong: ” + e.getMessage());
}
Better, but it suffers from the same problem to a lesser extent — without a stack trace, it can be very difficult to determine where an exception was thrown, just from the message.
The following code is simpler and much better:
try {
…
} catch (Exception e) {
throw new MyException(“something went wrong: ”, e);
}
Here we’ve captured the stack trace, so for no extra effort, we have significantly more information about what went wrong.
A corollary of this is catching exceptions that are never thrown — the unnecessary code to handle these just adds clutter and serves no purpose.
You should consider how likely an error condition is to occur and how easy it is to debug — maybe you should log some additional information when you catch the exception to facilitate debugging ?
Does it make sense to create our own exception type MyException or would an existing exception type describe the exception equally well ? If we create our own exception, does it make sense to extend something more specific than Exception or RuntimeException ?
Masking errors
Another anti-pattern I saw recently is masking errors — making them harder to detect. The code was something like this:
void doSomething(String someArgument) {
if (someArgument == null) {
return;
}
…
}
In this case, someArgument should probably never have been null and if it was null it might have caused other problems which would be difficult to debug, so it would have been better to omit this check and throw a NullPointerException if someArgument is null — it would be easier to debug.
If it had made sense to return instead of throwing, for example in safety-critical code where throwing an exception may not be an option, then the code should have logged an error message or returned some value indicating failure, instead of silently failing with no indication that anything had gone wrong.
Conclusion
Although exceptions are designed to simplify the process of error handling, it still requires thought to do well. Rather than the not-uncommon “she’ll be right” hope for the best attitude that some programmers seem to have, errors require the same level of consideration as the ‘happy path’ where everything goes right. Handling errors well can save users and developers a lot of frustration when, not if, things go wrong.