Thanks to
https://today.java.net/article/2006/04/04/exception-handling-antipatterns#antipatterns
Basic Exception Concepts
One of the most important concepts about exception handling to understand is that there are three general types of throwable classes in Java: checked exceptions, unchecked exceptions, and errors.
Checked exceptions are exceptions that must be declared in the throws clause of a method. They extend Exception and are intended to be an "in your face" type of exceptions. A checked exception indicates an expected problem that can occur during normal system operation. Some examples are problems communicating with external systems, and problems with user input. Note that, depending on your code's intended function, "user input" may refer to a user interface, or it may refer to the parameters that another developer passes to your API. Often, the correct response to a checked exception is to try again later, or to prompt the user to modify his input.
Unchecked exceptions are exceptions that do not need to be declared in a throws clause. They extend RuntimeException. An unchecked exception indicates an unexpected problem that is probably due to a bug in the code. The most common example is a NullPointerException. There are many core exceptions in the JDK that are checked exceptions but really shouldn't be, such as IllegalAccessException and NoSuchMethodException. An unchecked exception probably shouldn't be retried, and the correct response is usually to do nothing, and let it bubble up out of your method and through the execution stack. This is why it doesn't need to be declared in a throws clause. Eventually, at a high level of execution, the exception should probably be logged (see below).
Errors are serious problems that are almost certainly not recoverable. Some examples are OutOfMemoryError, LinkageError, and StackOverflowError.
Creating Your Own Exceptions
Most packages and/or system components should contain one or more custom exception classes. There are two primary use cases for a custom exception. First, your code can simply throw the custom exception when something goes wrong. For example:
throw new MyObjectNotFoundException("Couldn't find object id " + id);
Second, your code can wrap and throw another exception. For example:
catch (NoSuchMethodException e)
{
throw new MyServiceException("Couldn't process request", e);
}
Wrapping an exception can provide extra information to the user by adding your own message (as in the example above), while still preserving the stack trace and message of the original exception. It also allows you to hide the implementation details of your code, which is the most important reason to wrap exceptions. For instance, look at the Hibernate API. Even though Hibernate makes extensive use of JDBC in its implementation, and most of the operations that it performs can throw SQLException, Hibernate does not expose SQLException anywhere in its API. Instead, it wraps these exceptions inside of various subclasses of HibernateException. Using the approach allows you to change the underlying implementation of your module without modifying its public API.
Antipatterns
Log and Throw
catch (NoSuchMethodException e)
{
LOG.error("Blah", e);
throw e;
}
catch (NoSuchMethodException e)
{
LOG.error("Blah", e);
throw new MyServiceException("Blah", e);
}
catch (NoSuchMethodException e)
{
e.printStackTrace();
throw new MyServiceException("Blah", e);
}
All of the above examples are equally wrong. This is one of the most annoying error-handling antipatterns. Either log the exception, or throw it, but never do both. Logging and throwing results in multiple log messages for a single problem in the code, and makes life hell for the support engineer who is trying to dig through the logs.
Throwing Exception
public void foo() throws Exception {
This is just sloppy, and it completely defeats the purpose of using a checked exception. It tells your callers "something can go wrong in my method." Real useful. Don't do this. Declare the specific checked exceptions that your method can throw. If there are several, you should probably wrap them in your own exception (see "Throwing the Kitchen Sink" below.)
Throwing the Kitchen Sink
Example:
public void foo() throws MyException,
AnotherException, SomeOtherException, YetAnotherException{
Throwing multiple checked exceptions from your method is fine, as long as there are different possible courses of action that the caller may want to take, depending on which exception was thrown. If you have multiple checked exceptions that basically mean the same thing to the caller, wrap them in a single checked exception.
Catching Exception
try
{
foo();
}
catch (Exception e)
{
LOG.error("Foo failed", e);
}
This is generally wrong and sloppy. Catch the specific exceptions that can be thrown. The problem with catching Exception is that if the method you are calling later adds a new checked exception to its method signature, the developer's intent is that you should handle the specific new exception. If your code just catches Exception (or worse, Throwable), you'll probably never know about the change and the fact that your code is now wrong.
Destructive Wrapping
catch (NoSuchMethodException e)
{
throw new MyServiceException("Blah: " + e.getMessage());
}
This destroys the stack trace of the original exception, and is always wrong.
Log and Return Null
Example:
catch (NoSuchMethodException e)
{
LOG.error("Blah", e);
return null;
}
catch (NoSuchMethodException e)
{
e.printStackTrace();
return null;
}
Although not always incorrect, this is usually wrong. Instead of returning null, throw the exception, and let the caller deal with it. You should only return null in a normal (non-exceptional) use case (e.g., "This method returns null if the search string was not found.").
Catch and Ignore
Example:
catch (NoSuchMethodException e)
{
return null;
}
This one is insidious. Not only does it return null instead of handling or re-throwing the exception, it totally swallows the exception, losing the information forever.
Throw from Within Finally
Example:
try
{
blah();
}
finally
{
cleanUp();
}
This is fine, as long as cleanUp() can never throw an exception. In the above example, if blah() throws an exception, and then in the finally block, cleanUp() throws an exception, that second exception will be thrown and the first exception will be lost forever. If the code that you call in a finally block can possibly throw an exception, make sure that you either handle it, or log it. Never let it bubble out of the finally block.
Multi-Line Log Messages
Example:
LOG.debug("Using cache policy A");
LOG.debug("Using retry policy B");
Always try to group together all log messages, regardless of the level, into as few calls as possible. So in the example above, the correct code would look like:
LOG.debug("Using cache policy A, using retry policy B");
Using a multi-line log message with multiple calls to log.debug() may look fine in your test case, but when it shows up in the log file of an app server with 500 threads running in parallel, all spewing information to the same log file, your two log messages may end up spaced out 1000 lines apart in the log file, even though they occur on subsequent lines in your code.
Unsupported Operation Returning Null
Example:
public String foo()
{
// Not supported in this implementation.
return null;
}
When you're implementing an abstract base class, and you're just providing hooks for subclasses to optionally override, this is fine. However, if this is not the case, you should throw an UnsupportedOperationException instead of returning null. This makes it much more obvious to the caller why things aren't working, instead of her having to figure out why her code is throwing some random NullPointerException
