Exception Handling Question

Question

Hi All,

I have a question regarding exception handling. Consider following Java code snippet.

        try{
            //code
        }catch(SubSubException subsubex){
            //code
        }catch(SubException subex){
            //code
        }catch(Exception ex){
            //code
        }

I know this is the recommended way of handling exceptions. But I can achieve the same thing by using following code snippet.

        try{
            //code
        }catch ( Exception ex){
            if( ex instanceof SubException){              
                //code
            }else if(ex instanceof SubSubException){
                //code
            }else{
                //code
            }
        }

Can somebody tell me disadvantages of second approach?

Answer 1

The second approach is less readable. In addition Pokemon exception handling is never the way to go even though your "clever" trick is to use the instanceof keyword. I am not poking fun or mocking you in anyway, but it is best to write code for humans to read and maintain, not for the computer.

Answer 2

hmm, why do you even have to do the second approach? remember this, unless other options are better in terms of performance, readability, etc, you should stick with the conventions. catch statement were originally designed so they handle classification of exception types own their own so use them as is... just a thought!...

Answer 3

Considering the code in the first block tests the type of the exception, test for the base exception once (in the second bit of code you are testing for the base Exception twice in a way) and is less indented (thus less logic to grok), my thought is that the first one is far nicer, easier to understand etc.

Disadvantages: - Harder to understand

Answer 4

As an aside in re-ordering your "catching" of exceptions in your second example. If SubSubException extends SubException the second check will never be reached....

just something to be wary of when ordering your catches...

apart from that as mentioned elsewhere the question would have to be why try the second way when the first works, is the standard and is readable?

Answer 5

Second case is perfectly valid java code, but it has larger Cyclomatic complexity without adding any extra value.

Answer 6

The second approach is significantly less readable because:

• it requires more symbols,

• it requires deeper indentation,

• it is not idiomatic.

And IMO, the last is most important. You should be writing your code in a way that other Java programmers expect it to be written.

Answer 7

Yes, MadMurf points out the most important difference: reachability checking at compile-time. The standard idiom would catch something like this and rightfully prevent it from compiling:

    try {
    } catch (IndexOutOfBoundsException iooe) {
    } catch (ArrayIndexOutOfBoundsException aiooe) {
    }

The if/instanceof analog proposed in the original question would compile (which is NOT what you'd want because it's erroneous).

The reason why the standard idiom catches the error at compile time is given in JLS 14.21 Unreachable Statements.

• A catch block C is reachable iff both of the following are true:
  • [...]
  • There is no earlier catch block A in the try statement such that the type of C's parameter is the same as or a subclass of the type of A's parameter.

To further illustrate the point, the following compiles:

    try {
    } catch (Exception e) {
        if (e instanceof Exception) {
        } else if (e instanceof Exception) {
        }
    }

As you can see, this "pokemon catching" idiom is much harder to maintain because it circumvents some of the compile-time reachability check enforced in the standard idiom.

To make the point even more clear, whether you did it on purpose or not, you actually rearranged the order in which you checked the exceptions in your original question, a fact that could have easily been missed by others. If SubSubException is a subclass of SubException, the second if condition will NEVER be evaluated, and its body is effectively unreachable code.

The if/instanceof approach is VERY prone to mistakes.