33

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?

8 Answers 8

112

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.

11
  • 47
    never heard it refered to as pokemon exception handling - I like it. Feb 22, 2010 at 5:29
  • 10
    I can only hope it catches on :)
    – Woot4Moo
    Feb 22, 2010 at 5:31
  • 4
    I would like to know where "Pokemon exception handling" come from also... :o) I like it too!
    – jwwishart
    Feb 22, 2010 at 5:32
  • 4
    +1 for mentioning "best to write code for humans to read and maintain". So true. Feb 22, 2010 at 5:40
  • 29
    We should make "Pokemon exception handling" to an official anti-pattern!
    – Zappi
    Feb 22, 2010 at 11:34
41

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.

2
  • 12
    Others have made valid points, but this is the best answer. As programmers we should never be doing work that the compiler can do for us.
    – CurtainDog
    Feb 22, 2010 at 8:17
  • This should be accepted answer. The currently accepted answer only speaks about readability and style. Allowing compiler to check more is the correct answer. Feb 18, 2021 at 10:37
23

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!...

3
  • that was my mistake I misread what you said... I cant take it away though sorry :(
    – Woot4Moo
    Feb 22, 2010 at 5:51
  • @ultrajohn : make a dummy edit of your answer, so @woot4moo can remove his downvote :) Feb 22, 2010 at 8:12
  • @woot4, it's ok, i undertand:)
    – ultrajohn
    Feb 22, 2010 at 10:27
4

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

4

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.

2

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?

1

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

1

I think, better (much readable):

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

and

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.