I last previously talked about exception handling in a blog a couple of years ago:
Exception Handling. That was a fairly short blog which attempted to cure some of the more nefarious problems in exception handling which I sometimes see in code. Following these recommendations (nothing that isn't already very obvious) will result in "OK" code.
Now, I want to write up some more advanced guidelines such that following them will, in my humble opinion of course, improve "OK" code to "good" code.
I'm going to refer to the excellent
tutorial on Java Exceptions as I continue. You should definitely read and inwardly digest that material. I particularly recommend the section
Unchecked Exceptions -- the Controversy.
In the throwing and handling of exceptions, it seems to me that context is everything. This is why we sometimes wrap one exception in another -- because it allows us to add context. But there's another reason to wrap an exception. Here's a common situation:
import java.security.GeneralSecurityException;
import java.sql.Blob;
import java.sql.SQLException;
import javax.crypto.Cipher;
import org.apache.derby.iapi.jdbc.BrokeredConnection;
import org.apache.derby.iapi.jdbc.BrokeredConnectionControl;
public class MyConnection extends BrokeredConnection {
public MyConnection(final BrokeredConnectionControl bcc, final Cipher cipher) throws SQLException {
super(bcc);
this.cipher = cipher;
}
public Blob createBlob() throws SQLException {
return new EncryptedBlob(this.cipher) {
public byte[] getBytes(final long pos, final int length) throws SQLException {
final byte[] data = new byte[length];
// Fill in the actual data from somewhere
try {
return this.cipher.doFinal(data, (int) pos, 0);
} catch (final GeneralSecurityException e) {
throw new SQLException("crypto problem with cipher "+this.cipher
+ ", length: " + length, e);
}
}
};
}
Cipher cipher;
}
public abstract class EncryptedBlob implements Blob {
protected Cipher cipher;
/**
* @param cipher the cipher to use for encryption/decryption
*
*/
public EncryptedBlob(Cipher cipher) {
super();
this.cipher = cipher;
}
public long length() throws SQLException {
return 0;
}
// etc. etc.
}
Here, we are extending the (Apache) Derby
Connection implementation to allow for encryption/decryption of blobs. The details aren't important. But note the signature of the getBytes() method in the blob implementation. It throws a
SQLException. But when we try to perform encryption, we are going to have to deal with a
GeneralSecurityException. We have no choice about whether to
catch or
specify: we must catch it. We could eat the exception but that wouldn't be very good (see previous blog)! But since we have the ability to throw a
SQLException, we will do just that: wrap the caught exception inside a
SQLException. This of course also gives us the opportunity to provide some context: in this case, we don't want to pass back actual data which would be potentially insecure but, since the cipher details and the length of the byte array are quite likely to be relevant, we add those in.
What happens when we are implementing a method that doesn't throw an exception? An example of this is in the
ActionListener interface, the
actionPerformed(ActionEvent e) method.
new ActionListener() {
public void actionPerformed(ActionEvent e) {
// call method that throws a checked exception
}
};
The problem here is that we can't specify the exception in the method signature and we can't wrap the exception in a checked exception. We have to either handle it somehow, or throw it as a
RuntimeException. That's OK. It essentially will therefore treat whatever exception is thrown as a programming (logic) error. In other words, by the time the exception bubbles up to a potential handler, we really won't be able to handle it unless we specifically catch
RuntimeExceptions. And then we would have to look to see if the cause was possible to be handled. This doesn't really make good sense.
However, I should also note that, as beauty is in the eye of the beholder, so too an exception is a programming (logic) error according to the programmer. Suppose you parsing a
String as a
Number. If the
String was created by code, then the exception is probably justified as a
RuntimeException (which it is:
NumberFormatException extends
RuntimeException). But what if this
String was read from a file or the user just typed it in. That's not a logic error. Now, we want it to be a checked exception because we must handle it somehow. So, I'm not sure that the line between checked and unchecked exceptions is quite as clear as the tutorial suggests. In this particular case, for example, the Java designers seem to have got it wrong.
If you find yourself wrapping an exception of type A in a new exception of type A, then you almost certainly shouldn't be doing it. You wrap for necessity (as above) or context. Unless there's significant context to add, you should probably leave the exception alone and let it be passed up the stack.
Now, I want to talk about handling exceptions by logging. Let's say you decide to catch an exception rather than passing it up as is. You must
handle the exception by one of the following:
- performing some other logic based on the information that an exception was thrown [for example converting a String to a Number: you first call Integer.parse(x) and if that throws a NumberFormatException you instead try Double.parse(x)].
- wrapping it in a new exception (as described above).
- logging it.
In the last case, you are asserting that it's OK to continue. Meanwhile, for the purpose of improving the product you have kept a record of the incident and, if it was caused by a bug you have, in the logs, the stack trace to help in debugging.
But you shouldn't do more than one of these things. Think of it this way: there shouldn't be more than one reference to the exception. We should never, for example, find an exception has been logged twice. In the following code, the
try/catch in
doMain() is completely unnecessary. And it results in two copies of the exception going forward. Bad practice.
public class X {
public void doSomethingUseful() throws Exception {
throw new Exception("problem");
}
protected void doMain() throws Exception {
try {
doSomethingUseful();
} catch (final Exception e) {
logger.log(Level.WARNING, "something bad", e);
throw new Exception("wrapped", e);
}
}
private static Logger logger = Logger.getLogger(X.class.getName());
public static void main(final String[] args) {
final X x = new X();
try {
x.doMain();
} catch (final Exception e) {
e.printStackTrace();
}
}
}
If you are writing UI code and an exception bubbles up from below, then it makes sense to do the following:
- log it; and
- if appropriate, tell the user what went wrong (in terms the user will understand, which is generally not the way exceptions are created) and what he/she can do about it.
Finally, I want to strongly suggest the following rules (which I will not attempt to justify):
- There should be no more than one try/catch/finally block in any one method. Parallel to this rule is that there should be no more than one loop clause in a method (and ideally only one if construct).
- Don't bother to catch any exceptions in a private method unless you are going to do something really useful with it.
- As much as possible (Java 1.7 is good here) bunch the catch clauses together.
- Be careful only to catch the types of exception that you really want to handle (and can handle) -- don't for example specify Exception in a catch clause because you will end up catching RuntimeExceptions and you won't know if it's safe to proceed. It's OK to catch a superclass (e.g. GeneralSecurityException) but Exception is just too generic.
- User try-with-resource when appropriate (Java 1.7).
OK, back to work!
