views:

229

answers:

8

I've got a coding standards meeting in just over an hour and I need a quick answer to this one.

Common wisdom among experienced Java programmers is that you don't throw or catch java.lang.Exception (with rare exceptions - no pun intended). The reason you don't do this is that the statement

catch (java.lang.Exception ex) {...}

will also catch unchecked Exceptions, and in most cases this is not what is intended.

We already have a lot of legacy code written by existing team members where they catch a subclass of java.lang.Exception, log an error, and rethrow the subclass as java.lang.Exception.

I need to convince them that

  1. They need to stop writing code like this.
  2. Existing code that uses this anti-pattern needs to be fixed

Number 2 means a fair amount of refactoring.

It will shorten the argument at the meeting if I can show an article or blog entry by one of the Java community heavyweights that makes this point (i.e. Joshua Bloch, James Gosling). My google-fu hasn't turned up anything so far.

Does anyone know of an article or blog by a respected Java guru that says that you shouldn't throw or catch java.lang.Exception?

Quick answers are much appreciated.

Dean

A: 

I guess the point is that if you don't know how to handle a specific exception then you shouldn't catch it. It should propagate to a higher level in the hopes that it might know what to do with it. Catching exceptions too early leads to exceptions being swallowed silently and makes it impossible to do anything useful with them beyond logging.

Imagine what would happen if FileInputStream's constructor were to log an exception internally if you tried opening a file that did not exist, but didn't indicate this failure to your code. It's fine that the error gets logged but your code would like to catch this exception and do something useful with it (such as prompting the user for a new filename). If they were to catch (Exception) you wouldn't be able to do this.

Gili
A: 

If you have a copy of Josh Bloch's Effective Java to hand, I know he covers exception handling in a fair bit of detail there. I don't have access to it at present so I can't summarise it or give you page references, but I'm pretty sure he's got some good arguments for not catching java.lang.Exception.

Andrzej Doyle
That would be chapter 9 (17 pages). He doesn't prefer either one, but stresses the fact that you should only used checked exceptions if the caller can do something about the problem. So OOM is a RuntimeExc., while NoMoreElements is a (bad) checked exc.
Aaron Digulla
+1  A: 

Here is Bruce Eckel's viewpoint on checked exceptions in general, which is that in most cases they are a bad idea. Maybe that will have something that you can use.

Kaleb Brasee
+2  A: 

There is no common ground here. You will find two groups:

  1. Those who hate checked exceptions will catch and wrap them in a RuntimeException of some kind.

  2. Those who hate RuntimeException

The first group hates to riddle their code with try...catch, especially since in most cases, you can't handle the exception when you first see it. Think IOException: It could be thrown for every byte that your read. What's the low level code to do about it? It has to rethrow it so someone higher up can add some useful context to the error (like which file you were reading).

The other group wants to see what could go wrong. RuntimeException effectively hides this.

There is a simple fix, BTW: Make Exception extend RuntimException. Insane? Not really. If you do that with JDK 7, you get two compile errors.

The next step would be to have all Java compilers enumerate all runtime exceptions and list them in the throws entry in the class file. You still don't have to catch them but now, you'll know which ones could happen.

Lastly, extend the IDEs to display this. And presto, both groups could be happy. Unfortunately, this won't happen.

Aaron Digulla
A little rambling, but I like it!
Avi Flax
+5  A: 

Here's something: Java Tip 134: When catching exceptions, don't cast your net too wide (JavaWorld)

Effective Java (Second Edition) by Joshua Bloch might have something on this in Chapter 9 (Exceptions), although I couldn't quickly find anything about not catching Exception.

Here is a Q&A from JavaWorld about the question (also points to Java Tip 134) - it also explains why you sometimes have to break the rule of not catching Exception or even Throwable.

Jesper
+4  A: 

See this article from Brian Goetz (the concurrency wizard) who has his own insight as well as quoting Josh Bloch in Effective Java

Kevin
+1  A: 

where they catch a subclass of java.lang.Exception, log an error, and rethrow the subclass as java.lang.Exception. I need to convince them that they need to stop writing code like this.

I agree they need to stop coding like that, but for different reasons. There's not much sense in catching an exception just to log it and rethrow it.

An alternative is: don't catch the exception and let some higher up code (like a Java EE Filter, or try/catch in your main() method) catch and log all uncaught exceptions. Then you ensure each exception is only logged once, and you know all uncaught exceptions will be logged.

If you need to add extra information to the exception, catch it, change the message and rethrow it. I usually use a RuntimeException for this:

} catch (Exception originalException) {
    throw new RuntimeException("user name was " + userName, originalException);
}
Brad Cupit
Exceptions should be logged as close to where they were thrown as possible. It's possible that somewhere upstream the Exception will be swallowed, or another Error or RuntimeException will occur (e.g. NullPointerException) will occur that will prevent upstream logging.Did you mean for your catch() statement above to catch RuntimeException too?
Dean Schulze
@Dean Schulze presumably, if an exception was swallowed, that means it was not an unexpected exception, but it was an expected one and the code knew how to programatically handle it. Another way of stating it: whoever handles the exception should log it. If it goes unhandled all the way up, you should have some mechanism for handling and logging those unexpected exceptions. This ensures the stack trace only appears once in the logs.
Brad Cupit
@Dean Schulze as per the catch statement catching Exception, yeah, I was trying to show that you don't need to log the error and log variable values when the exception is caught. Instead, you can catch the exception and add those values to the message, then wrap the exception, and rethrow it. Your extra data still gets in the logs (and the stack trace is only output once, which makes it easier to search the logs for errors).
Brad Cupit
A: 

it's a long article but this proposes you:

  1. use checked exceptions when the client expects this error to occur regularly and must handle it (ex: user-entered data did not pass validation)
  2. use unchecked exceptions for unexpected conditions (ex: DB server went down)

In theory, #1 (expected errors) should have their own exception type, and the only place that should catch a #2 (a RuntimeException) is some type of top-level handler that catches the exception, logs it, and shows the user an error message, and even here, you should probably catch Throwable to make sure any uncaught exceptions are handled.

Following these guidelines, you shouldn't catch Exception since it doesn't fit either of the above criteria (meaning, it's not a RuntimeException and it's not a specialized Exception sublcass to indicate an expected error).

Brad Cupit