Dismiss Notice
Join Physics Forums Today!
The friendliest, high quality science and math community on the planet! Everyone who loves science is here!

C/++/# Why are Pokemon Exceptions considered code smell?

  1. Nov 26, 2016 #1
    "Gotta catch 'em all"

    e.g.

    Code (Text):

    try {
      // something that might fail for reasons I don't want to anticipate
    }
    catch (Exception ex) {
        // do something
    }
     
    I believe there are justifications for doing this:

    • Inside the try block I'm using a class from a 3rd party library that doesn't have any documentation. I'm not sure exactly what exceptions might be thrown and for what reason.
    • I'm aware of at least 5 types of exceptions that could be thrown and I would handle them each the same way. It seems rather stupid to write
    Code (Text):

    catch (ExceptionType1 ex) {
       // do something
    }
    catch (ExceptionType2 ex) {
       // do same thing as before
    }
     
     
  2. jcsd
  3. Nov 26, 2016 #2

    jtbell

    User Avatar

    Staff: Mentor

    A Google search for "c++ catch all exceptions" led me to some pages (e.g. this one) that give this code:

    Code (C):

    try {
      // something that might fail for reasons I don't want to anticipate
    }
    catch (...) {
        // do something
    }
     
     
  4. Nov 26, 2016 #3

    jedishrfu

    Staff: Mentor

    Java has a notion of checked exceptions and unchecked exceptions. For checked exceptions you must add code to either catch the exception or indicate that the given method may throw this exception. As you can imagine your code gets riddled with exception check logic. This causes many programmers to handle the exception poorly when they don't quite know what to do about. By poorly I mean that they may ignore it.

    Unchecked exceptions like RuntimeExceptions can happen at anytime and so Java doesn't require you to catch them but instead percolates them to the top and then exit. You then have the option to catch it or not at the appropriate spot. You now have leaner code that handles exceptions better.

    One Java best practices technique is to capture checked exceptions and then recast them as RuntimeExceptions or as custom subclasses thereof, if there is no better recourse thus ending the Java compiler check tyranny and ensuring that no exception will be left behind. For large programs this strategy works well but not all Java programmers ascribe to this best practice.

    Here's a discussion on the tradeoffs:

    https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
     
    Last edited: Nov 27, 2016
  5. Nov 27, 2016 #4
    When the call of the 3rd party library fails, catching the most general exception,
    Code (Java):
    try {
      // something that might fail for reasons I don't want to anticipate
    }
    catch (Exception ex) {
        // do something
    }
    is not a bad idea, unless your system should behave differently when different exceptions are thrown. When a failure of the 3rd party method is considered to be an unexpected exception and you need to rethrow it, then why not omit the whole try-catch-block at all and let the system propagate the exception to the caller level automatically?
    Because in this situation you would pollute your higher level classes with implementation details of the lower level classes (= 3rd party library in this case) and this is not a good idea. So, here the general try-catch-construction above is even a "good" smell: Just wrap the low-level exception with a higher level exception that is more appropriate to your program. For example, when reading data from a data source and your 3rd party modul is some SQL library, propagation of SQLException's to your high-level classes might not be a good idea, because in future you may switch to some other kind of data source that does not know anything about SQL, but you have polluted your higher level modules with SQLExceptions. Wrapping the SQLException into an IllegalStateException, IllegalArgumentException ... or even a general RuntimeException (or something meaningful) is a better way to handle it.
     
Know someone interested in this topic? Share this thread via Reddit, Google+, Twitter, or Facebook

Have something to add?
Draft saved Draft deleted