Something I wrote seven years ago and I’m publishing now to see if learned anything since then.
This is part of a series where I review common wisdom about Java error handling. The series is in no particular order, but the first installment explains my categorization method.
Clean up with finally
Truth: high
Importance: high
Joshua Bloch explains the reasoning behind this in Effective Java as “strive for failure atomicity.” Whatever happens, clean up after yourself. Checked exceptions give us one of their rare benefits by just maybe reminding us to write finally blocks.
One way not to write a finally block, however, is like this:
try {
connection = DriverManager.getConnection("stooges");
// Snip other database code
} catch (SQLException e) {
throw new RuntimeException(e);
} finally {
try {
// Bad. Do not do this.
connection.close();
} catch (SQLException e) {
throw new RuntimeException(e);
}
}
If opening the connection throws SQLException, closing the connection throws NullPointer and obscures the original cause. Usually, people suggest wrapping a conditional around the connection.close() call to check for null, but that gets ugly fast and is easy to forget. Instead, follow the next rule.
Make try blocks as small as possible
Truth: high
Importance: medium
Consider this code that obscures which file could not open:
try {
// Smelly. Don't do this.
curly = new FileReader("Curly");
shemp = new FileReader("Shemp");
} catch (FileNotFoundException e) {
throw new RuntimeException("Whoopwoopwoopwoop", e);
} finally {
// The close method translates IOException
// to RuntimeException
if (curly != null) {
close(curly);
}
if (shemp != null) {
close(shemp);
}
}
We programmers wrap multiple lines in the same handler because we are lazy, but like the hare napping during the race, that laziness hurts us in the end; it forces us to reason much more carefully about the application state during cleanup.
Instead, handle the exception as close to its cause as possible:
try {
curly = new FileReader("Curly");
} catch (FileNotFoundException e) {
throw new RuntimeException("Missing: Curly", e);
}
try {
shemp = new FileReader("Shemp");
} catch (FileNotFoundException e) {
throw new RuntimeException("Missing: Shemp", e);
} finally {
close(curly);
}
close(shemp);
The benefits of the shrunken try block may not be obvious from this tiny example, but consider how you can now factor out the file opening blocks to a method that simply throws an unchecked exception for missing files. Once done, this code simplifies down to:
curly = open("Curly");
try {
shemp = open("Shemp");
} finally {
close(curly);
}
close(shemp);
Note that all these examples lose the original exception when the close method throws an exception. Most applications can afford that minimal risk, but if you believe it likely that your cleanup code will throw further exceptions, a log and re-throw might be appropriate.
Do not rely on getCause()
Truth: high
Importance: high
Peeking at an exception’s cause is equivalent to using something’s privates or parsing the string representation to find a field value. It makes code brittle; moreover, you can only test it by forcing errors in your collaborators.
If you find that some library forces you to do this, consider avoiding that function completely; if you still have no way around it, adorn your code liberally with “XXX” comments, and test as best you can.
Do not catch top-level exceptions
Truth: low
Importance: low
Top-level exception classes like Exception live close to the root of the exception hierarchy. The argument against catching these says that you should avoid it because the specific lower type, such as FileNotFound, traditionally conveys information necessary to handling the exception, so by catching the top-level exception, your handler is dealing with an unknown error, which it probably knows little about.
Actually, the advice should say “do not try to recover from top-level exceptions.” Catching top-level exceptions is not fundamentally wrong or even bad, but because you do not really know how severe the exception was, you should usually do no more than report and organize a crash.
Should Be Simple 