2

I have the following structure in the legacy codebase:

try{
...
}
catch(Type1&){
...
}
catch(Type2&){
...
}
...

And with copy-paste development, the same catch blocks show up everywhere. Now, I would write a function like this:

void catchErrors(){
  try{
    throw;
  }
  catch(Type1&){
    ...
  }
  ...
}

and put it into the code like this:

try{
  ...
}
catch(...){
  catchErrors();
}

Will this be a valid refactor, resulting in the same functionality?
(And do you have any better suggestion for the refactor?)

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
Barney Szabolcs
  • 11,846
  • 12
  • 66
  • 91
  • How will that catch the exception object? Also where's the variable of the type you specified? – deW1 Oct 05 '15 at 09:59
  • 2
    [Test it and see?](http://coliru.stacked-crooked.com/a/f7ff38b04d7e6a17) – Lightness Races in Orbit Oct 05 '15 at 09:59
  • Two thoughts, not answers. Why not make it a const ref while you're there. 2nd, what are you trying to achieve - if some things need to catch one error type, and other bit of code, others perhaps this needs more thought. Part answer - if the "..." in the first code sample didn't contain `catch(...)` you have changed the behaviour. It will now silently swallow some exceptions. – doctorlove Oct 05 '15 at 10:03
  • @Lightness, I think the OP rather wants to catch 42 inside bar() – SingerOfTheFall Oct 05 '15 at 10:03
  • @SingerOfTheFall: [What do you think my example does?](http://coliru.stacked-crooked.com/a/c2fc88ca8a5f89c8) Perhaps you're not aware that `throw` without arguments is a "re-throw" operation. – Lightness Races in Orbit Oct 05 '15 at 10:03
  • @Lightness, well how do you suggest to get the exception object itself? `current_exception` is an option, but what if the op can't use c++11 (he mentioned legacy code, so who knows). – SingerOfTheFall Oct 05 '15 at 10:06
  • @SingerOfTheFall: Click on the link in my previous comment. It takes you to a full, working demonstration where you can see for yourself how it's done. – Lightness Races in Orbit Oct 05 '15 at 10:06
  • @LightnessRacesinOrbit yes, that's what I do now, trying different cases like objects, int, char*, I'm not sure I'm covering every case and it will work right. Also, c++ I know for seemingly working right, but you change some optimization flags or compile order or whatnot and then it breaks if you don't keep to the magic rules (such as EffectiveC++). – Barney Szabolcs Oct 05 '15 at 10:08
  • 1
    @BarnabasSzabolcs: True ;) Standard wording is conclusive (now provided). But showing you've tested it is a good first step :P – Lightness Races in Orbit Oct 05 '15 at 10:10
  • for bonus points... `#define TRY(X) do { try X catch(...) { catchErrors(); } } while (0)` , usage `TRY({ ...code... });` – M.M Oct 05 '15 at 10:34

2 Answers2

5

Yes, that's valid.

[C++14: 15.1/8]: A throw-expression with no operand rethrows the currently handled exception (15.3). The exception is reactivated with the existing exception object; no new exception object is created. The exception is no longer considered to be caught; therefore, the value of std::uncaught_exception() will again be true.

[ Example: code that must be executed because of an exception yet cannot completely handle the exception can be written like this:

try {
  // ...
} catch (...) { // catch all exceptions
  // respond (partially) to exception
  throw; // pass the exception to some
  // other handler
}

—end example ]

[C++14: 15.1/9]: If no exception is presently being handled, executing a throw-expression with no operand calls std::terminate() (15.5.1).

Although the throw-expression has been moved into its own function, during its execution an exception is still being handled, so it still works:

#include <iostream>

void bar()
{
    try {
        throw;
    }
    catch (int x) {
        std::cerr << "Damn " << x << "!\n";
    }
}

void foo()
{
    try {
        throw 42;
    }
    catch (...) {
        bar();
    }
}

int main()
{
    foo();
}

// Output: Damn 42!

(live demo)

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
3

Yes, your refactoring is valid. In fact, it is a fairly old technique, designed precisely to move the set of exception handlers into another function, and allow their reuse.

Note that calling your CatchErrors() will call std::terminate() if it is called outside of an exception handling block. That is required from a throw; statement if no exception is being handled.

Just don't rely on the technique too much. It is better to design the majority of your functions with some exception safety guarantees (i.e. they will not malfunction if an exception is thrown and they don't call it). That allows more chance of centralising your exception handling (e.g. in main()) rather than having lots of distinct functions that must each process exceptions separately. Doing that reduces the need to reuse exception handling code, since most exceptions will be handled in only one place.

Peter
  • 35,646
  • 4
  • 32
  • 74