views:

2380

answers:

10

I have always been of the belief that if a method can throw an exception then it is reckless not to protect this call with a meaningful try block.

I just posted 'You should ALWAYS wrap calls that can throw in try, catch blocks.' to this question and was told that it was 'remarkably bad advice' - I'd like to understand why.

Thanks!

+96  A: 

A method should only catch an exception when it can handle it in some sensible way.

Otherwise, pass it on up, in the hope that a method higher up the call stack can make sense of it.

As others have noted, it is good practice to have an unhandled exception handler (with logging) at the highest level of the call stack to ensure that any fatal errors are logged.

Mitch Wheat
Agree completely, but I would add that it is usually a good idea to have a top-level exception handler in `main()` (or other high-level function) that will log or output some sort of error message before the program dies.
Kristopher Johnson
It's also worth noting that there are costs (in terms of generated code) to `try` blocks. There's a good discussion in Scott Meyers's "More Effective C++".
Nick Meyer
Actually `try` blocks are free in any modern C compiler, that information is dated Nick. I also disagree about having a top-level exception handler because you lose locality information (the actual place where the instruction failed).
Blindy
@Blindly: the top exception handler is not there to handle the exception, but in fact to shout out loud that there was an unhandled exception, give its message, and end the program in a graceful way (return 1 instead of a call to `terminate`). It's more of a safety mechanism. Also, `try/catch` are more or less free when there isn't any exception. When there is one propagating it does consumes time each times it's thrown and caught, so a chain of `try/catch` that only rethrow isn't costless.
Matthieu M.
@Kristopher: Absolutely agree about the top-level logging. Just don't let the program continue running after you have logged the error! The program should die with a core dump at this point.
John Dibling
@Kristopher this has some advantages, but also has a terrible drawback. You have lost the stack from where the exception has been raised. If you can handle the exception use a try-catch but no use it to avoid the program terminate if you don't know how to solve the issue.
Vicente Botet Escriba
I thought it was obvious that I meant for the program to terminate after the "top-level handler" logs the exception info. Regarding "losing the stack": don't you lose it anyway if the program terminates due to an exception? C++ isn't like Java: it can't give you a pretty stack trace when it dies (unless you are debugging or your platform has special support for that sort of thing).
Kristopher Johnson
I disagree you should always crash on an uncaught exception. Modern software design is very compartmentalized, so why should you punish the rest of the application (and more importantly, the user!) just because there was one error? Crashing the the absolutely last thing you want to do, at the very least try to give the user some small window of code that will let them save work even if the rest of the application cannot be accessed.
Kendall Helmstetter Gelner
I also agree but I would also add that functions should still keep exceptions in mind even if they don't catch or even rethrow. There are three exception guarantees that you'll want to think about and probably document as you write any function.
Noah Roberts
Kendall: If an exception gets to a top-level handler, your application is by definition in an undefined state. Although in some specific cases there might be value to preserving the user's data (Word's document recovery comes to mind) the program shouldn't overwrite any files or commit to a database.
Hugh Brackett
`try` blocks certainly aren't free on all compilers. Specifically, the 32-bit MSVC compiler adds a fair amount of overhead just for a `try` block (I've seen tests pegging this at about 10-15% of the app's total runtime). Some other compilers use another exception implementation which avoids the runtime overhead, but in exchange uses much more memory. Either way, there's a cost, even when an exception is never thrown.
jalf
@jalf: I was under the impression that it's the throw that is (relatively) expensive. If no exception is thrown the overhead is extremely low.
Mitch Wheat
@Mitch: Depends on your definition of "extreme". :) It depends on how the compiler implements it, and like I said, there are a couple of strategies depending on whether you want to sacrifice execution time or memory, but there is a fair amount of plumbing that must be set up to keep track of which exception handlers exist, where to jump to when an exception is thrown, how to unwind the stack and so on. Of course, it costs *extra* when the exception is actually thrown, but merely having a `try` block costs something too.
jalf
(But as usual, don't worry too much about it until profiling shows it to be a problem)
jalf
+18  A: 

Because the next question is "I've caught an exception, what do I do next?" What will you do? If you do nothing - that's error hiding and the program could "just not work" without any chance to find what happened. You need to understand what exactly you will do once you've caught the exception and only catch if you know.

sharptooth
+5  A: 

The best advice I've heard is that you should only ever catch exceptions at points where you can sensibly do something about the exceptional condition, and that "catch, log and release" is not a good strategy (if occasionally unavoidable in libraries).

Donal Fellows
However, catch, log, and rethrow can be a good stragegy.
KeithB
@KeithB: I'd consider it a second-best strategy. It's better if you can get the log written in another way.
David Thornley
@KeithB: It's a "better than nothing in a library" strategy. "Catch, log, deal with it properly" is better where possible. (Yeah, I know it's not always possible.)
Donal Fellows
+5  A: 

As stated in other answers, you should only catch an exception if you can do some sort of sensible error handling for it.

For example, in the question that spawned your question, the questioner asks whether it is safe to ignore exceptions for a lexical_cast from an integer to a string. Such a cast should "never" fail. If it did fail, something has gone terribly wrong in the program. What could you possibly do to recover in that situation? It's probably best to just let the program die, as it is in a state that can't be trusted. So ignoring the exception may be the "safest" thing to do.

Kristopher Johnson
+8  A: 

Herb Sutter wrote about this problem here. For sure worth reading.
A teaser:

"Writing exception-safe code is fundamentally about writing 'try' and 'catch' in the correct places." Discuss.

Put bluntly, that statement reflects a fundamental misunderstanding of exception safety. Exceptions are just another form of error reporting, and we certainly know that writing error-safe code is not just about where to check return codes and handle error conditions.

Actually, it turns out that exception safety is rarely about writing 'try' and 'catch' -- and the more rarely the better. Also, never forget that exception safety affects a piece of code's design; it is never just an afterthought that can be retrofitted with a few extra catch statements as if for seasoning.

Tadeusz Kopec
+10  A: 

You don't need to cover every block with try-catches because a try-catch can still catch unhandled exceptions thrown in functions further down the call stack. So rather than have every function have a try-catch, you can have one at the top level logic of your application. For example, there might be a SaveDocument() top-level routine, which calls many methods which call other methods etc. These sub-methods don't need their own try-catches, because if they throw, it's still caught by SaveDocument()'s catch.

This is nice for three reasons: it's handy because you have one single place to report an error: the SaveDocument() catch block(s). There's no need to repeat this throughout all the sub-methods, and it's what you want anyway: one single place to give the user a useful diagnostic about something that went wrong.

Two, the save is cancelled whenever an exception is thrown. With every sub-method try-catching, if an exception is thrown, you get in to that method's catch block, execution leaves the function, and it carries on through SaveDocument(). If something's already gone wrong you likely want to stop right there.

Three, all your sub-methods can assume every call succeeds. If a call failed, execution will jump to the catch block and the subsequent code is never executed. This can make your code much cleaner. For example, here's with error codes:

int ret = SaveFirstSection();

if (ret == FAILED)
{
    /* some diagnostic */
    return;
}

ret = SaveSecondSection();

if (ret == FAILED)
{
    /* some diagnostic */
    return;
}

ret = SaveThirdSection();

if (ret == FAILED)
{
    /* some diagnostic */
    return;
}

Here's how that might be written with exceptions:

// these throw if failed, caught in SaveDocument's catch
SaveFirstSection();
SaveSecondSection();
SaveThirdSection();

Now it's much clearer what is happening.

Note exception safe code can be trickier to write in other ways: you don't want to leak any memory if an exception is thrown. Make sure you know about RAII, STL containers, smart pointers, and other objects which free their resources in destructors, since objects are always destructed before exceptions.

AshleysBrain
+48  A: 

As Mitch and others stated, you shouldn't catch an exception that you do not plan on handling in some way. You should consider how the application is going to systematically handle exceptions when you are designing it. This usually leads to having layers of error handling based on the abstractions - for example, you handle all SQL-related errors in your data access code so that the part of the application that is interacting with domain objects is not exposed to the fact that there is a DB under the hood somewhere.

There are a few related code smells that you definitely want to avoid in addition to the "catch everything everywhere" smell.

  1. "catch, log, rethrow": if you want scoped based logging, then write a class that emits a log statement in its destructor when the stack is unrolling due to an exception (ala std::uncaught_exception()). All that you need to do is declare a logging instance in the scope that you are interested in and, voila, you have logging and no unnecessary try/catch logic.

  2. "catch, throw translated": this usually points to an abstraction problem. Unless you are implementing a federated solution where you are translating several specific exceptions into one more generic one, you probably have an unnecessary layer of abstraction... and don't say that "I might need it tomorrow".

  3. "catch, cleanup, rethrow": this is one of my pet-peeves. If you see a lot of this, then you should apply Resource Acquisition is Initialization techniques and place the cleanup portion in the destructor of a janitor object instance.

I consider code that is littered with try/catch blocks to be a good target for code review and refactoring. It indicates that either exception handling is not well understood or the code has become an amœba and is in serious need of refactoring.

D.Shawley
#1 is new to me. +1 for that. Also, I'd like to note a common exception to #2, which is if you're designing a library often you'll want to translate internal exceptions into something specified by your library interface to reduce coupling (this may be what you mean by "federated solution", but I am not familiar with that term).
rmeador
Basically what you said: http://www.parashift.com/c++-faq-lite/exceptions.html#faq-17.13
Space_C0wb0y
+1 I agree completely with all the answer.
Vicente Botet Escriba
+4  A: 

The advice my computer science professor gave me once was: "Use Try and Catch blocks only when it's not possible to handle the error using standard means."

As an example, he told us that if a program ran into some serious issue in a place where it's not possible to do something like:

int f()
{
    // Do stuff

    if (condition == false)
        return -1;
    return 0;
}

int condition = f();

if (f != 0)
{
    // handle error
}

Then you should be using try, catch blocks. While you can use exceptions to handle this, it's generally not recommended because exceptions are expensive performance wise.

Sagekilla
That is one strategy, but many people recommend **never** returning error codes or failure/success statuses from functions, using exceptions instead. Exception-based error handling is often easier to read than error-code-based code. (See AshleysBrain's answer to this question for an example.) Also, always remember that many computer-science professors have very little experience writing real code.
Kristopher Johnson
-1 @Sagelika Your answer consists in avoiding exception, so no need of try-catch.
Vicente Botet Escriba
@Kristopher: Other big disadvantages to the return code is that it's real easy to forget to check a return code, and just after the call is not necessarily the best place to handle the problem.
David Thornley
+2  A: 

If you always handle exceptions immediately in the caller of a method that can throw an exception, then exceptions become useless, and you'd better use error codes.

The whole point of exceptions is that they need not be handled in every method in the call chain.

starblue
+2  A: 

I agree with the basic direction of your question to handle as many exceptions as possible at the lowest level.

Some of the existing answer go like "You don't need to handle the exception. Someone else will do it up the stack." To my experience that is a bad excuse to not think about exception handling at the currently developed piece of code, making the exception handling the problem of someone else or later.

That problem grows dramatically in distributed development, where you may need to call a method implemented by a co-worker. And then you have to inspect a nested chain of method calls to find out why he/she is throwing some exception at you, which could have been handled much easier at the deepest nested method.

Bananeweizen