Do you consider yourself a Java expert? Think you know everything about
exception handling? Can you quickly spot the six exception handling problems
below?
1: OutputStreamWriter out = ...
2: java.sql.Connection conn = ...
3: try { 5
4: Statement stat = conn.createStatement();
5: ResultSet rs = stat.executeQuery(
6: "select uid, name from user");
7: while (rs.next())
8: {
9: out.println("User ID : " + rs.getString("uid") + 6
10: ", name : " + rs.getString("name"));
11: }
12: conn.close(); 3
13: out.close();
14: }
15: catch(Exception ex) 2
16: {
17: ex.printStackTrace(); 1-4
18: }
Every Java developer should be able to spot at least two. If you can't
spot all six, read on.
Rather than provide general guidelines (most of which are well known),
we decided to reveal what we call anti-patterns: (unfortunately) common bad
programming practices that we've seen time and again in Java code. Our
purpose is to familiarize you with these counterexamples so you can quickly
spot them and avoid them.
Now for our hall of shame the six most common misuses and abuses of
exception handling.
Anti-pattern #1: try-catch-bury
15: catch(Exception ex)
16: {
17: ex.printStackTrace();
18: }
This is the bane of Java programming catching an exception and not
doing anything about it. In terms of frequency and severity, this is
probably comparable to the infamous unchecked buffer problem in C and C++.
If you see this anti-pattern, you can be 99% sure that it's wrong (there are
a few cases where it might make sense, but they should be carefully
commented to make it clear to the maintenance developer).
This is wrong because an exception (almost) always means that something
bad (or at least unexpected) has happened, yet we choose to ignore and
silence that cry for help. Calling printStackTrace does not qualify as
"handling an exception" it's okay for debugging but should not be present
past that initial step.
This problem is frighteningly pervasive. If you look at the
documentation for the JDK class ThreadDeath, you'll see the following
comment: "The class ThreadDeath is specifically a subclass of Error rather
than Exception, even though it's a normal occurrence,' because many
applications catch all occurrences of Exception and then discard the
exception." This poor programming practice is so common that, in an
unhealthy feedback loop, it has affected the design of Java itself. Grunt.
What should you do instead? You basically have four options:
1. Handle the exception: Do something about it. Correct the problem, notify someone, or do some different processing, whatever makes sense in this particular situation. Once again, calling printStackTrace does not
qualify as "handling the exception."
2. Rethrow the exception: This is not uncommon if your exception handling code examines the exception for additional information, and decides that it can't handle it after all.
3. Translate the exception into another exception: Most often this is used to translate a low-level exception into an application-level one.
4. Don't catch the exception: If you're not going to do anything about it, why bother catching it? Let it go (and, if necessary, declare it in the signature of your method).
Solution 1: If you catch an exception, do something about it. Don't bury it.
Anti-pattern #2: Pokémon gotta catch 'em all
15: catch(Exception ex)
It's often tempting to have a general catch statement. The most common
is undoubtedly catch(Exception ex). In almost all cases, that's a bad idea.
Why?
To understand why, we need to step back and consider the purpose of a
catch statement. You use a catch statement to indicate that you anticipate
certain exceptions and that you're willing to handle them. The class of the
exception(s) is how you tell the Java compiler which exceptions you want to
handle. Since the vast majority of exceptions inherit directly or indirectly
from java.lang.Exception, specifying that in your catch statement
effectively says you want to handle almost any exception.
Let's look at the code sample again. What kind of exceptions are we
likely to catch? The most obvious one is SQLException, quite likely to
happen any time you deal with JDBC. IOException is another possibility
we're dealing with a stream writer. You can already see the problem: it
doesn't make sense to handle these two (very different) types of errors in
the same catch block. It would make a lot more sense to have two catch
blocks, one for SQLException and one for IOException.
In addition, a large number of other exceptions could occur. What if,
for some obscure reason, executeQuery returns null? You'll get a
NullPointerException in the next line, yet that (unexpected) exception will
be ignored (unless you happen to be looking at the console for your program
and even then, unless you wrote that code yourself, what good does that do
you?).
The better approach is to be more specific. In this case, we should have
two catch blocks, one for SQLException, the other for IOException. This
makes it clear to the next reader of this code that you have thought about
these scenarios. But what about all the other exceptions? Let them go. Don't
handle them (unless you have a reason to). You cannot (and in fact should
not) try to handle all possible exceptions let them bubble up; someone
else will deal with them (the JVM if nothing else).
There are some instances where catching Exception is a good idea,
especially when testing or debugging. Once you have isolated the cause of
the problem, you need to remove the catch Exception and replace it with the
appropriate kind of Exception(s).
Solution 2: Be as specific as possible in your catch statements and use
multiple catch statements if necessary. Don't try to handle all possible
exceptions.
Anti-pattern #3: The forgotten bathroom key
3: try {
4: Statement stat = conn.createStatement();
5: ResultSet rs = stat.executeQuery(
6: "select uid, name from user");
7: while (rs.next())
8: {
9: out.println("User ID : " + rs.getString("uid") +
10: ", name : " + rs.getString("name"));
11: }
12: conn.close();
13: out.close();
14: }
One of us used to work in a small office with only one bathroom and you
had to use a key. One programmer was notorious for neglecting to return the
key to its rightful place, thereby inconveniencing the rest of the office.
Often this was because he would get interrupted while coming back to his
desk and would forget to return the valuable resource.
Similarly, when an exception occurs, it affects the execution path of
your program. It's easy to overlook that simple fact. If you use resources
such as files, sockets, JDBC connections, etc., you need to make sure
they're properly closed and released even if an exception occurs. Java has a
mechanism designed for such a thing: finally.
Finally is a wonderful thing; it allows you to guarantee that your
cleanup code will be executed before the end of the try/catch/finally block,
regardless of whether or not an exception is being thrown. Think of it as a
string on your finger that reminds you to return the bathroom key, even if
something distracts you. Yet finally is rarely used, even by intermediate
programmers.
Of course, finally blocks should be written with great care. In
particular, you should be careful about exceptions being thrown in a finally
block this is your final chance to clean things up; don't let it slip. In
some cases, it may be acceptable to bury an exception that's thrown in a
finally block. Think of it as a best effort. Regardless, use your best
judgment, or better yet, ask someone more experienced.
Solution 3: Make sure all resources are properly released. Use finally
liberally.
Anti-pattern #4: If you don't know why I am mad, I won't tell you
3: try {
4: Statement stat = conn.createStatement();
5: ResultSet rs = stat.executeQuery(
6: "select uid, name from user");
7: while (rs.next())
8: {
9: out.println("User ID : " + rs.getString("uid") +
10: ", name : " + rs.getString("name"));
11: }
12: conn.close();
13: out.close();
14: }
15: catch(Exception ex)
16: {
17: ex.printStackTrace();
18: }
Anyone who has been married for a number of years can relate to this
situation. Look at the code again: what happens if an exception occurs
during the execution of the loop? Will we get information that can tell us
what causes the loop to fail? Not really. All we'll get is an indicator that
something is wrong in the area of the class that's doing the processing, but
we may not get any input as to what caused the problem.
The basic "stack trace" that shows the path of execution to the class
that caused the exception is cryptic and not easily parsed. It also causes
any tester without a Java development background to report application
problems with a very general description such as "encountered an ugly Java
problem." If such an exception is wrapped to display a message in plain
English, testers are more likely to repeat the specific message displayed to
them. Not everyone who tests business applications is experienced in the
delicate art of stack trace interpretation.
In addition to displaying a more user-friendly message, there's great
benefit in providing a little extra data such as the Class, Method, and a
text message. While it's true this information can be obtained from the
stack trace, it's much easier to read when it's isolated. Also, taking the
extra time to describe the behavior when you're creating the code will
greatly expedite resolving the problem later.
One solution is to embed a snippet like the following one wherever
exceptions get logged in the application. You can use the
this.getClass().getName() method, and insert the method named and a text
message every time the program reports a checked exception. This practice
makes exceptions easy to read and to parse.
this.getClass().getName(), "mymethod", "some message"
One caveat to this approach is that with a static method call you need
to manually insert the class name because the "this" object reference does
not have a this to refer to. Simply replace the
this.getClass().getName()with your class name "myClass".
Solution 4: Report a reasonable amount of data in a way that's easy to read.
Anti-pattern #5: Overzealous try
3: try {
4: Statement stat = conn.createStatement();
5: ResultSet rs = stat.executeQuery(
6: "select uid, name from user");
7: while (rs.next())
8: {
9: out.println("User ID : " + rs.getString("uid") +
10: ", name : " + rs.getString("name"));
11: }
12: conn.close();
13: out.close();
14: }
Putting too much activity in a single try block is a bad programming
practice, even though it's common. The reason it's common is that it takes
extra time to isolate what could go wrong in a particular block and what
exceptions could get created as a result of this processing. Wrapping too
many statements in a giant catch is like preparing for a move by packing
your household goods in refrigerator-sized boxes. Sure, all your goods are
in one place, but imagine the sorting task you'll have in the near future.
For the inexperienced developer, it's easier to put a bunch of statements in
a try block and wrap it up in a generic catch Exception than to isolate the
statements. This practice makes troubleshooting Exceptions generated during
program execution that much more difficult because there are more variations
to check to find out what caused the Exception.
The code example doesn't really illustrate this fully because there's
not enough space to show a complete example, but this is a common problem.
Solution 5: Keep try blocks as small as possible, but no smaller.
Anti-pattern #6: Incomplete execution
7: while (rs.next())
8: {
9: out.println("User ID : " + rs.getString("uid") +
10: ", name : " + rs.getString("name"));
11: }
This is the silent killer of Java systems. Look at the code again and
imagine what happens if an exception is thrown in the middle of the loop.
The execution of the loop will be interrupted, the catch block will be
executed, and that's about it. What about the data that we're writing out?
It will be silently incomplete. Whoever or whatever is supposed to use this
data will receive incomplete (and hence incorrect) data, but will probably
have no idea that it is incomplete. For some systems, that's a problem far
more serious than any crash.
A better approach here would be to attempt to write a message to the
output, signaling that the data could not be completely written. Another
possibility would be to buffer the output (if possible) and write it out
once we have it ready to go.
Solution 6: Consider all likely exceptions and how they'll affect the flow
of execution.
Example (Rewritten)
Given these facts, Listing 1 shows what the same code should look like. Note that it's quite a bit more verbose. That's not a bad thing good code often
includes a lot of error handling.
Conclusion
All the anti-patterns in this article were inspired by actual code
written by professional programmers. Before you snicker, ask yourself
whether your code is really free from these poor practices. Even the most
experienced programmers sometimes fall back into them, simply because
they're easy (we certainly do). Think of them as bad habits they are
notoriously difficult to shake off, but just trying makes you a better
person.
Of course, nothing in this article should be taken as gospel. Use common
sense and your experience. For each anti-pattern described here, you could
certainly come up with counterexamples. As long as the rules are broken
willingly and thoughtfully, you have our blessing, but we do beg for one
favor: if what you do is not obvious, please write a nice comment. You'll
thank yourself later. Also, share your wealth of knowledge with others on
your team; extra time spent on training is a small price to pay in
comparison to a massive refactoring effort at the end of the development
cycle.
References
Haggar, P. (2000). Practical Java: Programming Language Guide.
Addison-Wesley.
Ambler, S.W., et al. (2000). The Elements of Java Style. Cambridge
University Press.
The Sun Java online tutorial:
http://java.sun.com/docs/books/
tutorial/essential/exceptions
Bloch, J. (2001). Effective Java: Programming Language Guide.
Addison-Wesley.
Author Bios
Craig Dewalt is a surfer who likes to lead Java development teams.
goflyr@yahoo.com
Max Tardiveau is the founder of Integrity Logic.
max@tardiveau.com
"Rebel Without a Clause"
Vol. 8, Issue 3, p. 48
Listing 1
1: OutputStreamWriter out = ...
2: java.sql.Connection conn = ...
3: try {
4: Statement stat = conn.createStatement();
5: ResultSet rs = stat.executeQuery(
6: "select uid, name from user");
7: while (rs.next())
8: {
9: out.println("User ID : " + rs.getString("uid") +
10: ", name : " + rs.getString("name"));
11: }
12: }
13: catch(SQLException sqlex)
14: {
15: out.println("Warning : data incomplete due to exception");
16: throw new ApplicationException(
17: "SQL error while reading", sqlex);
18: }
19: catch(IOException ioex)
20: {
21: throw new ApplicationException(
22: "I/O error while writing", ioex);
23: }
24:finally
25:{
26: if (conn != null) {
27: try {
28: conn.close();
29: }
30: catch(SQLException sqlex2)
31: {
32: System.err(this.getClass().getName() +
33: ".mymethod - cannot close SQL connection : " +
34: sqlex2.toString());
35: }
36:
37: if (out != null) {
38: try {
39: out.close();
40: }
41: catch(IOException ioex2)
42: {
43: System.err(this.getClass().getName() +
44: ".mymethod - cannot close outfile" +
45: ioex2.toString());
46: }
47: }
48:}