3

I have a method which is getting an exception as a parameter to a particular method. This method needs to do perform different logic based on the type of exception. Between below two ways of handling this.. what is the most efficient way (is there a third better way).. and why

public void processException(Exception ex) {
   try {
      throw ex;
   } catch(CustomException e1) {
      //do something
   } catch (CustomException2 e2) {
      //do something else
   }
}

or

public void processException(Exception ex) {
   if (ex instanceof CustomException) {
      // do something
   } else if (ex instanceof CustomException2) {
      // do something else
   }
}
Dev Blanked
  • 8,555
  • 3
  • 26
  • 32
  • 4
    Exceptions are supposed to be, well... *exceptional*. Is this really a performance concern? – Mysticial Jul 31 '13 at 16:39
  • Why would you want to do that in the first place? Where are you calling that method from. Can you show some more code? – Rohit Jain Jul 31 '13 at 16:39
  • The former. However, writing a method for this sending an exception object is unnecessary. You could do this in the same place where it was originally created. – asgs Jul 31 '13 at 16:40
  • @Mysticial my concern is which out of the above two methods is more efficient and why – Dev Blanked Jul 31 '13 at 16:41
  • 1
    Either way will be around a micro-second, you shouldn't be call it enough for it to matter. – Peter Lawrey Jul 31 '13 at 16:41
  • 1
    @DevBlanked The point is, if you ever find yourself asking this question, you have done something wrong with your design. – Peter Lawrey Jul 31 '13 at 16:41
  • @asgs the exception can be raised in many places of the code base and in each of these places the above logic needs to be performed – Dev Blanked Jul 31 '13 at 16:42
  • @DevBlanked. Do you have any idea, how many types of exception you would need to test in this single method? Certainly it's a bad design. – Rohit Jain Jul 31 '13 at 16:43
  • If you just log or print the stack trace in those catch blocks I'd argue that you shouldn't catch them at all. You need to have a true handling strategy; otherwise just add throws to the method signature. – duffymo Jul 31 '13 at 16:46
  • 1
    The first approach is a bad idea. It's guaranteed to be slower, and will likely run afoul of some debuggers. All in all I see nothing to recommend it, other than perhaps that it allows you to demonstrate your cleverness. – Hot Licks Jul 31 '13 at 16:46
  • 1
    (To above commmenters: I interpret the question as implying that the exception (perhaps caught elsewhere) is passed in and operated on (perhaps to extract info to log). The above method's job is *not* to signal the exception.) – Hot Licks Jul 31 '13 at 16:50

4 Answers4

7

Efficiency aside, the second way is preferred, because the exception is not thrown in a non-exceptional situation. The first way to "dispatch" uses exception throwing in a regular control flow, which makes it harder to read your program.

Besides, the two methods are not identical: the first program must be declared as throwing a checked exception, because not all subtypes are handled by the catch blocks.

If you are dealing with custom exceptions that your program defines, you have a way to avoid checking the subtype: since Exception objects are regular classes, you could add a package-visible method to them, and have them implement a package-visible interface holding that method. Exceptions would then be able to override that method, letting you use a regular "virtual" dispatch rather than checking for the exact class type at runtime.

Here is an illustration of this approach: let's say you want your exceptions to write themselves to a log file. You can do this as follows:

interface LoggableException {
    void logToFile();
}
public class CustomExceptionOne extends Exception implements LoggableException {
    ...
    public void logToFile() {
        // Do something
    }
}
public class CustomExceptionTwo extends Exception implements LoggableException {
    ...
    public void logToFile() {
        // Do something else
    }
}
public void processException(Exception ex) {
    if (ex instanceof LoggableException) {
        LoggableException lEx = (LoggableException)ex;
        lEx.logToFile();
    }
}
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • dasblinkenlight, thx for the explanation. Having each exception responsible for the processing is certainly a good idea – Dev Blanked Aug 01 '13 at 04:54
4

The first one is a very bad idea. Logic should not be expressed as exception catching. They're expensive, whether you realize it or not.

I don't like the second one, either. When you see if/else logic, you should think about polymorphism.

duffymo
  • 305,152
  • 44
  • 369
  • 561
2

Obviously the second option is better. All the Exceptions are eventually objects and it is better, safe and recommended to perform instance of check to determine the type of object.

Ankur Shanbhag
  • 7,746
  • 2
  • 28
  • 38
1

Throwing and catching Exceptions are expensive operations and should only occur in exceptional circumstances. In addition, your method to handle the exception is probably being called because the exception was already thrown. I wouldn't throw it again.

Either instanceof, or overloaded processException like rgettman suggests. Another alternative is

public void processException(Exception e) {
    Class eClass = e.getClass();
    if (eClass == CustomeException.class) { // This exception is most likely.
        // do something
    } else if (eClass == CustomException2.class) {
        // do something
    } else if (eClass == CustomException3.class) {
        // do something
    } else if (eClass == CustomException4.class) {
        // do something
    }
}

I would suggest short circuiting the if/else if statements with the most likely exception class.

km1
  • 2,383
  • 1
  • 22
  • 27