Monday, November 26, 2007

Exception Handling Anti-patterns

Exception Handling Anti-patterns:

Rather than discussing best patterns, it is important to discuss anti-patterns for specific concepts, where such concepts are misunderstood most of the time. In this post, we will discuss anti-patterns in Exception Handling.

Once an Exception is thrown, then never catch it; Never catch it unnecessarily.

This is a thumb rule of exception handling. Once an exception is thrown, then it should be never caught, until it propagates to the top level caller. Otherwise, it will turn very difficult to trace the exception flow in the code. Exception should not be caught unless it is going to be consumed (ignored), or there is some code to be run in the catch block.

The following example can illustrate it.

private void methodA(String name) throws ABCException
{
methodB(name);
}
private void methodB(String name) throws ABCException
{
methodC(name);
}
private void methodC(String name) throws ABCException
{
if (name == null) //thrown here
throw new ABCException("Name arg can not be null.");

//else do something..
}

methodC() throws ABCException, but methodA() and methodB() do not catch the ABCException any where, they just allow the exception to propagate to their callers. In this example, the ABCException should not be caught in methodB() or methodA() unless it is necessary. It is very common mistake that developer unnecessarily catch exception.

Also, don’t catch the exception just for logging or printing the information; only the top level caller has to log the exception – other wise the same exception will be logged many times.

Never throw Runtime exceptions.

Any exception (or custom exception), that is a subclass of java.lang.RuntimeException is a runtime exception, i.e: NullPointerException, ArrayIndexOutOfBoundsException.

Because, the runtime exceptions occur in exceptional cases, that can not be handled. A program or an application should not throw any runtime exception. Any program that throws runtime exception is buggy. So, runtime exception should not be thrown at all from the code or it should not be caught in the code at all (let it propagate, so that you would come to know that it is a code bug).

Sample code snippet that catches a runtime exception.

try
{
quotient = numerator / denominator;
}
// This is bad, dont catch!
catch (ArithmeticException arithmeticException)
{
// some code goes here..
}


In the above example, there are two bad things!

1. The code should not catch the runtime exception (ArithmeticException)

2. The code should be written smart enough to avoid possible runtime exceptions; In this case, the program should validate the denominator before dividing – so that ArithmeticException will not occur.

The above code can be re-written as:

if (denominator == 0)
throw new XYZApplicationValidationException("Denomintor can not be null!");

quotient = numerator / denominator;

The second code snippet does not catch runtime exceptions and also avoids possible runtime exceptions.

Do not throw simply java.lang.Exception or Multiple checked exceptions.

Most common mistake in exception handling is, just throwing java.lang.Exception! If a method throws java.lang.Exception then all of it’s callers are forced to throw java.lang.Exception or wrap java.lang.Exception!; this is worst case, because it restricts the reusability of code. Other developers may hesitate to call this method as it is difficult to throw or wrap java.lang.Exception.

private void methodA(String name) throws Exception
{
//…..code
}

It is very hard to reuse methodA !

Also, a method throwing multiple checked exceptions may look tough! Instead of throwing multiple checked exceptions, the method can throw the parent of all those checked exceptions. It does not affect the exception flow, but same time, it makes the method declaration to look clean.
Ex:

public void methodA() throws BException, CException, DException
{}

In the above example, if AException is the parent of BException, CException and DException, the method declaration can be rewritten as,

public void methodA() throws AException
{}


The later version of method declaration looks pretty clean than the former.

When wrapping exceptions, the root exception info should not be lost.

When wrapping exceptions, it is important to copy all information of the root exception.

catch(ABCException abcException)
{
String rootExceptionMsg = abcException.getMessage();

throw new MyCustomException(rootExceptionMsg);
}

In the above example, MyCustomException wraps the root exception (ABCException). But, StackTrace of the ABCException is lost, any error code, any parameter and any other information of ABCException is also lost! Care should be taken to copy all info. Also, the wrapping exception class should be designed such that it stores the instances of the root exceptions (so that stack trace and other info can also be stored).

Careful design of Custom Exception.

  • Custom exception can be a runtime exception unless it is necessary to be a checked exception. Because, all the callers have to declare it or handle it or wrap it.
  • It is good to store instances of all root exceptions in custom exception (though it adds weight to custom exception instance, this will be useful to dig exception later).
  • Do not create separate exception class for each error scenario. Too much of exception classes will be hard to maintain.
  • Pass all necessary parameters in exception message.

Write code such that it will not lead to runtime exception.

Code should be written such that it will not lead to any runtime exceptions. NullPointerException is the most common runtime exception we face. Let’s discuss the common runtime exception cases.

  • Before accessing any attribute or calling any method on an instance, do a null check.

if (str1.equals(str2))
//...

Above ‘if’ condition will lead to NullPointerException, if str1 is null. It can avoid NullPointerException, if the code is rewritten as,

if ( str1 !=null && str1.equals(str2))
//…..

  • Do not return null from a method, return zero length array instead

public String [] getNames()
{
return null;
}

The caller of getNames() may not do a null check, so it is better to return zero length array also instead of returning null.

public String [] getNames()
{
return new String[0]
}

No comments: