Java exception handling idioms ... who's right and how to handle it?

Question

I currently have a technical point of difference with an acquaintance. In a nutshell, it's the difference between these two basic styles of Java exception handling:

Option 1 (mine):

try {
...
} catch (OneKindOfException) {
...
} catch (AnotherKind) {
...
} catch (AThirdKind) {
...
}

Option 2 (his):

try {
...
} catch (AppException e) {
    switch(e.getCode()) {
    case Constants.ONE_KIND:
    ...
    break;
    case Constants.ANOTHER_KIND:
    ...
    break;
    case Constants.A_THIRD_KIND:
    ...
    break;
    default:
    ...
    }
}

His argument -- after I used copious links about user input validation, exception handling, assertions and contracts, etc. to back up my point of view -- boiled down to this:

"It’s a good model. I've used it since me and a friend of mine came up with it in 1998, almost 10 years ago. Take another look and you'll see that the compromises we made to the academic arguments make a lot of sense."

Does anyone have a knock-down argument for why Option 1 is the way to go?

Answer 1

When you have a switch statement, you're less object oriented. There are also more opportunities for mistakes, forgetting a "break;" statement, forgetting to add a case for an Exception if you add a new Exception that is thrown.

I also find your way of doing it to be MUCH more readable, and it's the standard idiom that all developers will immediately understand.

For my taste, the amount of boiler plate to do your acquaintance's method, the amount of code that has nothing to do with actually handling the Exceptions, is unacceptable. The more boilerplate code there is around your actual program logic, the harder the code is to read and to maintain. And using an uncommon idiom makes code more difficult to understand.

But the deal breaker, as I said above, is that when you modify the called method so that it throws an additional Exception, you will automatically know you have to modify your code because it will fail to compile. However, if you use your acquaintance's method and you modify the called method to throw a new variety of AppException, your code will not know there is anything different about this new variety and your code may silently fail by going down an inappropriate error-handling leg. This is assuming that you actually remembered to put in a default so at least it's handled and not silently ignored.

Answer 2

I don't know if I have a knock down argument but initial thoughts are

• Option 2 works until your trying to catch an Exception that doesn't implement getCode()
• Option 2 encourages the developer to catch general exceptions, this is a problem because if you don't implement a case statement for a given subclass of AppException the compiler will not warn you. Ofcourse you could run into the same problem with option 1 but atleast option 1 does not activly encourage this.

Answer 3

With option 1, the caller has the option of selecting exactly which exception to catch, and to ignore all others. With option 2, the caller has to remember to re-throw any exceptions not explicitly caught.

Additionally, there's better self-documentation with option 1, as the method signature needs to specify exactly which exceptions are thrown, rather than a single over-riding exception.

If there's a need to have an all-encompassing AppException, the other exception types can always inherit from it.

Answer 4

• the way option 2 is coded, any unexpected exception type will be swallowed! (this can be fixed by re-throwing in the default case, but that is arguably an ugly thing to do - much better/more efficient to not catch it in the first place)
• option 2 is a manual recreation of what option 1 most likely does under the hood, i.e. it ignores the preferred syntax of the language to use older constructs best avoided for maintenance and readability reasons. In other words, option 2 is reinventing the wheel using uglier syntax than that provided by the language constructs.

clearly, both ways work; option 2 is merely obsoleted by the more modern syntax supported by option 1

Answer 5

I'd support option 2 if it was:

default:
  throw e;

It's a bit uglier syntax, but the ability to execute the same code for multiple exceptions (ie cases in a row) is much better. The only thing that would bug me is producing a unique id when making an exception, and the system could definitely be improved.

Answer 6

The knock-down argument would be that it breaks encapsulation since I now I have to know something about the subclass of Exception's public interface in order to handle exceptions by it. A good example of this "mistake" in the JDK is java.sql.SQLException, exposing getErrorCode and getSQLState methods.

Answer 7

1. Unnecessary have to know the code and declare constants for the exception which could have been abstract when using option 1.

2. The second option (as I guess) will change to traditional (as option 1) when there is only one specific exception to catch, so I see inconsistencey over there.

Answer 8

It looks to me like you're overusing exceptions in either case. As a general rule, I try to throw exceptions only when both of the following are true:

1. An unexpected condition has occurred that cannot be handled here.
2. Somebody will care about the stack trace.

How about a third way? You could use an enum for the type of error and simply return it as part of the method's type. For this, you would use, for example, Option<A> or Either<A, B>.

For example, you would have:

enum Error { ONE_ERROR, ANOTHER_ERROR, THIRD_ERROR };

and instead of

public Foo mightError(Bar b) throws OneException

you will have

public Either<Error, Foo> mightError(Bar b)

Throw/catch is a bit like goto/comefrom. Easy to abuse. See Go To Statement Considered Harmful. and Lazy Error Handling

Answer 9

I think it depends on the extent to which this is used. I certainly wouldn't have "one exception to rule them all" which is thrown by everything. On the other hand, if there is a whole class of situations which are almost certainly going to be handled the same way, but you may need to distinguish between them for (say) user feedback purposes, option 2 would make sense just for those exceptions. They should be very narrow in scope - so that wherever it makes sense for one "code" to be thrown, it should probably make sense for all the others to be thrown too.

The crucial test for me would be: "would it ever make sense to catch an AppException with one code, but want to let another code remain uncaught?" If so, they should be different types.

Answer 10

Use both.

The first for most of the exceptions in your code.

The second for those very "specific" exceptions you've create.

Don't struggle with little things like this.

BTW 1st is better.

Answer 11

Each checked Exception is an, um, exception condition that must be handled for the program behavior to be defined. There's no need to go into contractual obligations and whatnot, it's a simple matter of meaningfulness. Say you ask a shopkeeper how much something costs and it turns out the item is not for sale. Now, if you insist you'll only accept non-negative numerical values for an answer, there is no correct answer that could ever be provided to you. This is the point with checked exceptions, you ask that some action be performed (perhaps producing a response), and if your request cannot be performed in a meaningful manner, you'll have to plan for that reasonably. This is the cost of writing robust code.

With Option 2 you are completely obscuring the meaning of the exception conditions in your code. You should not collapse different error conditions into a single generic AppException unless they will never need to be handled differently. The fact that you're branching on getCode() indicates otherwise, so use different Exceptions for different exceptions.

The only real merit I can see with Option 2 is for cleanly handling different exceptions with the same code block. This nice blog post talks about this problem with Java. Still, this is a style vs. correctness issue, and correctness wins.