6

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

Dean Schulze
  • 9,633
  • 24
  • 100
  • 165
  • 1
    just want to show you that question: http://stackoverflow.com/questions/237585/java-lang-exception-vs-rolling-your-own-exception Some nice answers in it – bastianneu Oct 16 '09 at 15:02
  • I read that one before posting this. A couple of the answers and comments say the same thing, but I'm looking for an authority. This will result in a fair amount of refactoring so it would help to have an expert's opinion to back me up. – Dean Schulze Oct 16 '09 at 15:45

9 Answers9

6

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
  • 202,709
  • 46
  • 318
  • 350
5

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

Kevin
  • 30,111
  • 9
  • 76
  • 83
4

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
  • 321,842
  • 108
  • 597
  • 820
  • A little rambling, but I like it! – Avi Flax Oct 16 '09 at 15:01
  • People who like checked exceptions don't hate runtime exceptions, they just limit their use to the situations where they're appropriate. – nairbv May 14 '13 at 19:07
  • @Brian: My argument is that there is no way to agree when when they are "appropriate". `SQLException` and `IOException` are clearly runtime exceptions in my view since in my experience, they are thrown more often because of a programming error rather than because of a condition that the caller could actually handle. – Aaron Digulla May 15 '13 at 13:22
  • yeah, I consider SQLException in particular to be the main reason there's so much misunderstanding about checked exceptions. As an API, they consider that you could be writing a client app in which the SQL code is user input. Then a SQL Exception really should be checked. DB password and connectivity issues should also be checked. In practice though, most of us are writing web apps. We consider the SQL queries (and related exceptions) to be bugs in our code. I would not be happy with making Exception extend RuntimeException. I'd be happy if devs better understood when to use what. – nairbv May 15 '13 at 14:32
  • 1
    After 27 years in IT, I have given up on "devs should understand what they do" and turned towards "make it really easy to use correctly." – Aaron Digulla May 15 '13 at 15:01
3

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
  • 51,193
  • 8
  • 108
  • 113
2

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 should use another tactic, 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
  • 6,530
  • 8
  • 55
  • 60
  • 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 Oct 16 '09 at 19:10
  • @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 Oct 19 '09 at 18:06
  • @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 Oct 19 '09 at 18:10
1

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
  • 6,530
  • 8
  • 55
  • 60
  • 1
    The URL to the article referenced [has changed](http://www.oracle.com/technetwork/articles/entarch/effective-exceptions-092345.html). @TerrelShumway posted the same article in his answer. – hotshot309 Dec 21 '12 at 15:31
  • People who like checked exceptions don't hate runtime exceptions, they just limit their use to the situations where they're appropriate. – nairbv May 13 '13 at 17:43
  • @Brian maybe you meant to post this on [Aaron Digulla's answer](http://stackoverflow.com/a/1578723/152061)? – Brad Cupit May 13 '13 at 22:00
  • @bradCupit: oop! not sure how that happened. – nairbv May 14 '13 at 19:07
1

Here is a very insightful article

Summary:

  1. Use checked exceptions for rare but valid contingencies that relate to the purpose of your code. Client code should know how to handle these.

  2. Use unchecked exceptions for faults that shouldn't happen but do. (Server down, classpath misconfigured, SQL syntax error, etc.) The IT guy who manages the server, or the developer who just passed bad SQL to prepareStatement() should know how to fix these problems. Faults should propagate up to the logging layer so the info doesn't get lost.

Terrel Shumway
  • 454
  • 3
  • 6
0

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
  • 86,244
  • 97
  • 390
  • 689
0

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
  • 102,507
  • 33
  • 189
  • 228
  • 1
    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 Oct 16 '09 at 15:03