Function-wide exception handling in c++ - is it a bad style?

Question

There is a try-catch thing about functions, which I think sometimes may be quite useful:

bool function() 
try
{
    //do something
}
catch(exception_type & t)
{
   //do something
}

So the first part of the question: is this style considered bad in general case?

And the concrete example I used this approach in:

We had project with quite a lot of code in c and c++. And there we had custom exception types (not std::exception derived). I needed to integrate XML library and cast all exception to our types. So, basically, the last step was to catch all exceptions from XML library and convert them.

Function before:

bool readEntity(...)
{
   while(...)
   { 
      if(...)
      {
         //lot's of code...
      }
   }
}

after:

bool readEntity(...)
try
{
   while(...)
   { 
      if(...)
      {
         //lot's of code...
      }
   }
}
catch(XMLBaseException & ex)
{
    //create our exception and throw
}

My thoughts went something like this: I clearly state my intentions to convert all exception derived from one type into custom type AND we keep our screen without horizontal scroll bar (cause horizontal scroll bars are bad).

Well, I actually was quite criticized for this approach as for non-clear one during code review.

So I'd like to hear you thoughts.

UPDATE: just to be clear: refactoring the function wasn't an option. And actually it was good written one.

Answer 1

Really the only reason to function-level try blocks is for constructors, otherwise it's a somewhat obscure feature that doesn't buy you that much. It's just as easy to do it this way:

bool readEntity(...)
{
   try
   {
      while(...)
      { 
         if(...)
         {
            //lot's of code...
         }
      }
   }
   catch(XMLBaseException & ex)
   {
       //create our exception and throw
   }
}

If you are having troubles with horizontal scrolling then the thing to do is to split up your code. try/catches are complexity and this should be represented in the nesting level, not hidden.

In constructors, this is a different issue: there is no other way to catch exceptions in an initializer list:

SomeClass::SomeClass(parameter p1, parameter p2) : Member1(p1), Member2(p2)
try
{ 
}
catch(Exception &ex)
{
    // handle and rethrow
}

Of course, if you have an exception mid-construction, there's not likely much you can do to recover except log and and rethrow (it's going to get rethrown anyway in the constructor case). Your object isn't completely constructed yet and there's nothing you can really do with it. The only thing that you can trust to be valid are the parameters (although if the initialization failed, that will likely be due to bad parameters).

See this GOTW for a discussion on this.

Answer 2

Just to be clear: rethrowing and repackaging exceptions isn't poor practice, it's a Good Thing since it minimizes exposure of external dependencies.

However, function-level try-catch is intended for constructor initialization. Saving a couple of horizontal spaces in your code isn't worth using a relatively obscure language feature for. Better to refactor the nested code if you really are indenting that far!

Answer 3

That function is supposed to return a bool so you can return a true or false but if the function were to fail and the bool was used for something other than success flag(which is bad practice unless pure c) it should throw to signal the caller it didn't work.

Answer 4

There is a good discussion about the general uselessness of constructor/function try blocks at Herb Sutters GotW site.