8

This is not a question about exception handling in general, but it applies specifically end exclusively to the use of some frameworks. A few examples of typical starting points:

  • GWT: public void onFailure(Throwable caught) implementation of the AsyncCallback interface.
  • JAX-RS: public Response toResponse(E throwable) implementation of the ExceptionMapper<E extends Throwable> interface.

Both the above methods receive an instance of Throwable. Normally, I've seen developers use a simple "if/else if" block to differentiate the handling logic:

// As specified by the AsyncCallback class of the GWT framework
public void onFailure(Throwable caught) {
    if (caught instanceof AnException) {
        // handle AnException
    } else if (caught instanceof AnotherException) {
        // handle AnotherException
    } else if (caught instanceof YetAnotherException) {
        // handle YetAnotherException
    } else if (caught instanceof ...) {
        // and so on...
    }
}

Since I am not a fan of "if/else if" blocks for many reasons, I came up with the following "pattern" which converts the "if/else if" block into a "try/catch" block, behaving as if it were a "switch" block:

public void onFailure(Throwable caught) {
    try {
        throw caught;
    } catch(AnException e1) {
        // handle AnException
    } catch(AnotherException e2) {
        // handle AnotherException
    } catch(YetAnotherException e3) {
        // handle YetAnotherException
    } catch(...) {
        // and so on...
    }
}

My question is: Are there any drawbacks - in terms of performance, best practices, code readability, general safety, or just anything else I'm not considering or noticing - using this approach?

Louis Bono
  • 178
  • 1
  • 10
  • why have the caught in the first place? – Stultuske Sep 18 '17 at 13:45
  • As stated in the premise of my question, I am using a framework, and adapting the code to what the framework requires. – Louis Bono Sep 18 '17 at 13:46
  • 2
    Possible duplicate of [Is it expensive to use try-catch blocks even if an exception is never thrown?](https://stackoverflow.com/questions/16451777/is-it-expensive-to-use-try-catch-blocks-even-if-an-exception-is-never-thrown) – derOtterDieb Sep 18 '17 at 13:48
  • In my opinion, there is nothing wrong with this. If you have a parent class ``caught`` that contains multiple exceptions handlers, and they need to be logged differently or something. Then this is how I would do it. – Jeremy Sep 18 '17 at 13:51
  • I'm not that experienced, and mostly know C# and Java 8 (meaning I wasn't around for the dark ages of java), but I've only ever seen or done the 2nd style. If I saw the 1st if/else style, I would be annoyed. – Novaterata Sep 18 '17 at 14:01
  • There are pros and cons. Basically, this is about using exceptions for control flow (which is generally frowned upon). – Klitos Kyriacou Sep 18 '17 at 14:04
  • 1
    An alternative would be to put the different logic into the Exception type itself (if they are your own), so that you could have a single clause. In fact, I would recommend that if most of the Exceptions are your own checked exceptions. A lot of if/else/switch behavior is better off being polymorphism – Novaterata Sep 18 '17 at 14:05
  • @KlitosKyriacou I am not exactly using exceptions for control flow. I am using a framework that forces me to handle a generic `Throwable`, for which I need to check the subclass in order to handle each exception type separately. – Louis Bono Sep 18 '17 at 14:10
  • 1
    @Novaterata Yes, that case is handled using the JAX-RS framework, where you can create a specific `ExceptionMapper` implementation to handle your own checked exceptions. But sometimes frameworks will force you to handle only `Throwable` (like GWT for example). – Louis Bono Sep 18 '17 at 14:13
  • @kazu Thanks. That helps me understand the performance part. I was also trying to address a broader scope with my question. – Louis Bono Sep 18 '17 at 14:20
  • 1
    I think it looks fine. A small drawback is if you forget a "catch-all" clause and send the error back "into" the framework from the error handling code. Could lead to a hard debugging session one day. – Henrik Aasted Sørensen Sep 18 '17 at 14:21
  • Another dupe: https://stackoverflow.com/questions/17966265/what-is-the-cost-of-try-catch-blocks – jannis Sep 18 '17 at 15:08
  • And another one: https://stackoverflow.com/questions/2184935/performance-cost-of-coding-exception-driven-development-in-java – jannis Sep 18 '17 at 15:12
  • @LouisBono but you are using exceptions for flow control. Your are using a try-catch as a decision-making statement. This is the definition of "exception driven development". Found one more dupe: https://softwareengineering.stackexchange.com/questions/189222/are-exceptions-as-control-flow-considered-a-serious-antipattern-if-so-why – jannis Sep 18 '17 at 15:20
  • @jannis I don't think this is a dupe of those. This question was in comparing a single catch with if/else on type as opposed to multiple catches per type. All of those questions are concerned with Exceptions in general. – Novaterata Sep 18 '17 at 15:22
  • you are basically asking is it ok to use exceptions for flow control. the last link I provided points to "Are exceptions as control flow considered a serious antipattern? If so, Why?" post. if this is not a dupe then I don't know what is – jannis Sep 18 '17 at 15:32
  • 1
    @jannis The intention is not flow control, but exception handling. If I weren't using the mentioned frameworks, I wouldn't even bother to post such a question. The frameworks require me to work according to their own rules, and so I need to adapt. I edited my question adding bold text to point out what I'm looking for. – Louis Bono Sep 18 '17 at 16:38
  • You are (re)throwing an exception for the sole purpose of selecting which branch of code to follow, so yes, this is an exemplar of exceptions-for-control-flow. As to whether your particular implementation is 'okay', I wouldn't agonize over it, as you'll only wind up on this code path when something has already gone wrong, and that should be uncommon to begin with. You're also not filling in a new stack trace, so that's one less expensive thing going on. – Mike Strobel Sep 18 '17 at 23:26
  • @MikeStrobel As mentioned previously, I am within the boundaries of a framework (GWT, JAX-RS, etc.) which is "doing the exception handling for me" as part of the framework, but it really isn't. If I weren't using these frameworks, I wouldn't need to post this kind of question. – Louis Bono Sep 18 '17 at 23:31

3 Answers3

5

Using exceptions to direct program flow under normal circumstances is a code smell, but that's not really what you're doing here. I think you can get away with this for a few reasons:

  1. We already catch and re-throw exceptions for all manner of reasons (e.g., "catch, take some action, propagate"). This is a bit different in intent, but it's no worse in terms of cost.

  2. You've already incurred the cost of this exception being thrown at least once. You've possibly incurred the cost of its causes being thrown, caught, and wrapped or re-thrown. The cost of filling in the stack traces has already been paid. Re-throwing an already-populated exception one more time is not going to increase the order of complexity.

  3. You are not using exceptions to direct the flow of the normal code path. You're reacting to a error, so you are already on the exceptional path, and you should rarely (if ever) end up here. If this pattern is inefficient, it will hardly matter unless you are encountering lots of exceptions, in which case you have bigger problems. Spend your time optimizing the paths you expect to take, not the ones you don't.

  4. Aesthetically, there are few things that make my skin crawl like long chains of if/else if blocks, especially when the conditions are merely type-checking. What you're proposing is, in my opinion, far more readable. Having multiple, ordered catch clauses is common, so the structure is mostly familiar. The try { throw e; } preamble may be unorthodox, but it's easy enough to reason about.

Just be wary when propagating Throwable. Some errors, like the VirtualMachineError hierarchy, are a sign that something has gone horribly wrong, and they should be allowed to run their course. Others, like InterruptedException, communicate something about the state of the originating thread, and they should not be blindly propagated on a different thread. Some, like ThreadDeath, span both categories.

Mike Strobel
  • 25,075
  • 57
  • 69
  • 'Using exceptions for flow control' is a tautology. They *are* flow control. What this principle originally applied to was using exceptions as a kind of `goto` within a method. It does actually apply to this case, but not to most cases where it is cited (`EOFException` for example). – user207421 Sep 19 '17 at 00:49
  • @EJP Yes, exceptions are always flow control. But they are intended for _exceptional_ flow control, not _normal_ flow control. Using them direct program flow in _non-exceptional_ circumstances is a code smell, but that's not what OP is doing. – Mike Strobel Sep 19 '17 at 12:10
2

Performance would only matter if there are a huge amount of errors thrown. It doesn't impact the performance in the success case. There being so many errors would be much more of an issue than the time is takes to process them.

If you call a local method, and it throws an exception, then it would be fine to use catch blocks to process it. This is doing the same but with remote methods. It's not normal control flow because an exception has already been thrown from the RPC call before getting to the method, so it's fine to use the usual exception handling constructs.

There are some checks that can be done by the compiler, like checking that the most specific exceptions are listed first, but the use of the general Throwable type loses some type safety. This is unavoidable because of the framework.

I would be fine with either example here, as it doesn't make much of a difference.

fgb
  • 18,439
  • 2
  • 38
  • 52
0

The two blocks of code you show are actually superficially very similar: they are both the same "shape" to my eye at first glance.

And it's worth noting that the if/else chain is actually fewer lines of code and more immediately comprehensible than the try/catch version.

I don't think the try/catch version is wrong per se, but when compared side by side like this I don't see any reason why it would be better either.

And all else being equal, uncontroversial code is always better than controversial code: you never want a reader of your code to be distracted away from what your code is doing by how you've chosen to do it.

Daniel Pryden
  • 59,486
  • 16
  • 97
  • 135