Background
As a professional Java Developer, over the past decade or so, I've worked with all sorts of Java developers and I have had
the opportunity to maintain thousands of lines of second-hand Java code. One thing that continues to pop up, even to this day, is bad
exception handling. It is almost as if exception handling is a an afterthought to a lot of Java developers. A lot of times, I see
exceptions being treated as a nuisance, an inconvenience, a burden, akin to child-proof pill bottles (which are proven to be more
adult-proof than child-proof), and flashing VCR clocks (you youngsters out there surely have no idea what I'm talking about).
|
Add a little tequila...
...to your Java
|
Overview
In this article, I'll discuss three common exception handling mistakes, why they are mistakes, and the correct approach that should be
taken in each situation.
Common Mistake #1 - Using Exceptions as Conditionals
This mistake occurs when a developer uses a try/catch in a situation where a simple if/else statement would suffice. For
example, handling null values:
Object somevalue = ...
try
{
somevalue.doSomething(); //do the non-null condition
}
catch(NullPointerException npe)
{
...//do the null condition
}
Best Practice
For both performance and asthetics, the example above is better coded as:
Object somevalue = ...
if(somevalue != null)
{
somevalue.doSomething(); //do the non-null condition
}
else
{
...//do the null condition
}
It is important to note, though, that in some situations it is acceptable to use a try/catch for conditional coding, but it should be
limited to situations where the cost of determining the conditional state is greater than or equal to the cost of performing the
"normal-state" operation. For an example of this, see the integer parsing code in the "Best Practice" to Common Mistake #2 below.
Common Mistake #2 - Exception Dropping
No, this isn't what you do at a party to brag about all of the "famous" exceptions you know (think Name Dropping). Instead, this is the practice where a developer makes an (usually
wrong) assumption that an exception in a specific situation is irrelevant or unimportant, therefore choosing to catch the exception and
do absolutely nothing with it. For example:
try
{
//... do something that potentially throws an IOException
}
catch(IOException ioe)
{
}
In this example, should an IOException be thrown in the try clause, the catch clause traps the exception and then drops it. Should
something bad happen, the developer and/or user won't know about it, and the program will continue after the catch as if nothing has
happened.
Best Practice
There are actually two different solutions to this problem, but only one of them is correct in any given situation. The
developer must analyze the situation and choose the most correct approach. First, the developer should
determine if it is really necessary to catch the exception in the first place. It may actually be more reasonable to allow the
exception to propogate up the stack. A few questions to ask are, "Can I recover from this exception?", "Do I have an alternate
course of action should this exception occur?", and "Is this exception purely a local concern, or would the caller of my method be
interested in this exception?" The answer to these questions, may lead the developer to determine that the exception should be
propogated up the stack.
In the second situation, a developer needs to ask herself if it is genuinely reasonable for the exception to be dropped. There are
actually a few situations where it is perfectly reasonable to drop an exception. The following example illustrates a situation where
dropping an exception would be considered reasonable:
int intValue = 1; //if someValuePassedIn is invalid, default to 1
try
{
intValue = Integer.parseInt(someValuePassedIn);
}
catch(NumberFormatExcepion nfe)
{
//ignore NumberFormatException because we want to default to 1 if someValuePassedIn is invalid
}
In this example, the exception is dropped, but a comment is placed in the code to illustrate that the developer has at least thought
about the consequences of dropping the exception and has accepted the responsibility for doing so. It should also be noted that this
example seems to violate "Common Mistake #1" above. However, because it would be as costly, or more costly to try to determine if the
"someValuePassedIn" is a valid integer, it is better here to determine its validity as we parse it.
An alternative to this solution may be to log the exception stack trace for later use should the same type of exception be
unexpectedly thrown by another line of code within the try clause.
Common Mistake #3 - Exception Generalization
This is probably the most common and over-abused exception handling mistake I encounter. This is the practice where a developer will
declare a method's throws clause or a try's catch clause to throw or catch a java.lang.Exception or even worse, a java.lang.Throwable.
For example:
public void myMethod() throws Throwable
{
try
{
...//do something
}
catch(Exception e)
{
...//do something
}
}
This practice can have all sorts of nasty side effects. First of all, catching overgeneralized exceptions has two negative
consequences. One, it can cause the code to inappropriately handle an exception that the developer had no expectation of the code
throwing in the first place. RuntimeException is a perfect example of this phenomenon. By catching java.lang.Exception, due to the
inheritance model, a developer will also catch all RuntimeExceptions which are typically thrown when an unexpected logic condition has
occurred, usually as the result of a bug and/or coding issue. In the majority of the situations where a RuntimeException is thrown, the
correct approach is to log the exception and let the system die gracefully as to allow the developer to fix whatever logic condition
caused the problem, but by catching java.lang.Exception, depending on how it is handled, the developer may never know this logic
condition ever occurred.
The second negative consequence of catching an overgeneralized exception is that it can prevent the code calling a method from ever
realizing an exceptional state that it had an expectation or duty of handling. Catching java.lang.Throwable illustrates this point very
well. By catching Throwable, the catch clause will, by definition, also catch all java.lang.Error objects that are thrown. According
to Sun's documentation and by definition, "An Error is a subclass of Throwable that indicates serious problems that a reasonable
application should not try to catch." (see java.lang.Error).
So by catching Throwable, a developer is violating this convention and potentially catching Errors that in reality should be propogated
up to the JVM so that the JVM can die gracefully. Instead, by catching the Error, the JVM may enter an unpredictable state and become
very unstable. A good example of this phenomenon would be the java.lang.OutOfMemoryError. In earlier versions of Java,
catching an OutOfMemoryError would cause the VM to attempt to push a frame onto the stack which would in turn cause another
OutOfMemoryError to be thrown, which at best would cause the potential for stack trace information being lost (if any were
available to begin with) and at worst, depending on the code structure, could cause code to get caught in an infinite loop of
catching and throwing OutOfMemoryError.
Secondly, throwing overgeneralized exceptions has its own problems. The biggest problem of this practice is that subsequent developers
wishing to make calls to these methods will have no reasonable expectation of what exceptions may be thrown. This prevents developers
from being able to write defensive code, or write code that has any chance of reasonably dealing with any exception that occurs as a
result of invoking such a method.
Best Practice
Methods should only be declared to throw whatever sepcialized exceptions that can logically be expected to be thrown. Likewise, a
developer should only catch specialized exceptions that she has a reasonable expectation of handling. Also, even though it may seem
overly verbose and overly complicated it is better to have many specialized catch statements than a single generalized catch statement,
even if all of the specialized exceptions will be handled the same way. For example:
try
{
... //do something here that might cause several different types of exceptions to occur
}
catch(SomeExceptionA sea)
{
handle(sea);
}
catch(SomeExceptionB seb)
{
handle(seb);
}
catch(SomeExceptionC sec)
{
handle(sec);
}
catch(SomeExceptionD sed)
{
handle(sed);
}
private void handle(Exception e)
{
... //generically handle any Exception e
}
is preferrable over:
try
{
... //do something here that might cause several different types of exceptions to occur
}
catch(Exception e)
{
... //generically handle any Exception e
}
Even though it is more verbose and takes more effort to code, the first example eliminates any possibility of inadvertantly catching and
handling any excpetion that wasn't intended to be caught.
Summary
By avoiding the common mistakes illustrated in this article, and instead following the suggested best practices, a developer can make
their code more reliable, and execute more predictably, resulting in more efficient, less buggy, and more easily maintained code.
[an error occurred while processing this directive]
|
|