Good day

I'm looking for some guidance and/or opinions regarding good style when using try-catch blocks.  When (as is more often than not the case) the statement that may throw the exception is followed by other statements to be executed if and only if said exception is not thrown, I see a lot of this pattern:

try {
    //statement that may throw exception here
}
catch(Exception e) {
    //statements to deal with failure here
    return;
}
//statements to be executed if and only if exception is not thrown here

Particularly when there are multiple instances of this pattern in a method nested at various levels I'm not comfortable with the proliferation of exit points because it means that I can't trace an execution path through the method simply by reading branching/looping conditions and observing braces and indents - I now have to check every block for return statements.  Is it just me who has a problem with this?  Unfortunately all the alternatives I can think of are worse.  Any ideas?

Regards
me

 Isn't this the same "problem" as doing:

function asdf()
{
    // code...
    if (x) {
        return x;
    }

    // more code

    if (y) {
        return y;
    }

    // more code

    return z;

}

Return statements can be anywhere in the function body.  Your best bet is to get an editor that clearly highlights return statements.

catch(Exception e) {
    //statements to deal with failure here
    return;
}

This is bad form.  Your function should only catch the exception if it can actually deal with it and continue on successfully.  You could catch it, do some last minute cleanup and rethrow (or throw a different exception).  Though cleanup would be better placed in a finally block if your language supports it.  Doing a normal return implies non-exceptional execution.

The type of code above would make sense if you didn't want your exceptions crossing a module boundary.  Then you may have to return an error code instead.

Otherwise, you wouldn't catch at all (though depending on the language, you may need a finally block to do function cleanup).  You just let the exception wind up the stack.  So your function either succeeds or passes an exception.

In a language with destructors (destructors can clean up any allocations when an exception is thrown)

f()
{
  s1;
  s2;
  s3;
}

Without  destructors:

 f()
{
  try {
    s1;
    s2;
    s3;
  }
  finally {
    // clean up
  }
}

If you have an optional statement in your function, then you can do try/catch and handle the problem as you wish.  Otherwise let the upper levels decide what to do in the case of error. 

If you look at David Abrahams' exception safety guarantees (http://www.boost.org/community/exception_safety.html) he describes 3 level of exception safety:

basic: don't break the class and don't leak resources.
strong: roll-back semantics.  Put everything back the way it was.
no-throw:  function guarantees it will not throw

The basic guarantee is what I described above.  Clean up by using  RAII or finally clauses so nothing is leaked.  The strong guarantee is usually done with the create and swap method.  You create a temporary object with all the updated values.  If that does not throw, then following statements swap the contents with safe instructions (e.g. simple pointer assignment).  If there is an exception, the temporary will be removed either by its destructor or the garbage collector.

Not sure if this is obvious, but an exception should never be used where regular logic would work. Perhaps some of the try catch blocks can simply be removed in place of other conditionals.

Otherwise I agree I'm not a big fan of the return in the catch block -- although I do prefer the use of return in general over if/then or big if blocks.

chebrock:

Not sure if this is obvious, but an exception should never be used where regular logic would work.

A rule of thumb of mine is to only use exceptions if your function is unable to perform the actions it had promised to do. As already mentioned: only use catch if you can actually do something about it. Don't be afraid to use try{ } finally { } pattern a lot (without the catch): if an underlying function throws, you can clean up in the finally and you leave the exception handling to a higher layer.

bjolling:

chebrock:

Not sure if this is obvious, but an exception should never be used where regular logic would work.

A rule of thumb of mine is to only use exceptions if your function is unable to perform the actions it had promised to do. As already mentioned: only use catch if you can actually do something about it. Don't be afraid to use try{ } finally { } pattern a lot (without the catch): if an underlying function throws, you can clean up in the finally and you leave the exception handling to a higher layer.

True. I'd rather have a function throwing an exception than having it eat the exception silently. This is exactly the kind of situation t hat usually leads to TRUE, FALSE, FILE_NOT_FOUND situations.

DogmaBites:

Otherwise, you wouldn't catch at all (though depending on the language, you may need a finally block to do function cleanup).  You just let the exception wind up the stack.  So your function either succeeds or passes an exception.

 

This is all well and good until it isn't all well and good.  If you are writingGUI code for instance you can' just allow the exception to keep going up the stack unless you want your users to stop using your product.

tster:

DogmaBites:

Otherwise, you wouldn't catch at all (though depending on the language, you may need a finally block to do function cleanup).  You just let the exception wind up the stack.  So your function either succeeds or passes an exception.

 

This is all well and good until it isn't all well and good.  If you are writingGUI code for instance you can' just allow the exception to keep going up the stack unless you want your users to stop using your product.

You can't swallow your exception somewhere in between either. If your application doesn't produce the correct results and doesn't indicate somehow an exception has occured, users will stop using your product before your can say "bug report".

aassddff:

Particularly when there are multiple instances of this pattern in a method nested at various levels I'm not comfortable with the proliferation of exit points...

Nobody really answered this part of the OP's question, so I'll have a first go at it.

I think your question should be the other way around. Instead of nesting try-catch-finally blocks, I tend to create separate functions for simplicity's sake. Try-catch actually helps you (e.g. with many exit points) to exit cleanly under all circumstances when dealing with complex functions.

try{ someting that can fail; }

catch (the specific exception you can handle)  { ; }

finally { clean up your resources; }

return;

 

My 2 year old daughter also mentions:

rd ftyjtkiooplo414kh(-sesdfg

tster:

DogmaBites:

Otherwise, you wouldn't catch at all (though depending on the language, you may need a finally block to do function cleanup).  You just let the exception wind up the stack.  So your function either succeeds or passes an exception.

 

This is all well and good until it isn't all well and good.  If you are writingGUI code for instance you can' just allow the exception to keep going up the stack unless you want your users to stop using your product.

I'm sorry, I wasn't clear.  You should let it go up until you can actually deal with it.  Catching it in the middle, where you don't know what you should because of the exception is bad form.  It's usually the upper levels that know whether it should just be logged, throw up a dialog saying "Sorry Dave, I can't do that." or try an alternate method.

The OP looked like middle level code simply eating the exception and going on with life. 

DogmaBites:

tster:

DogmaBites:

Otherwise, you wouldn't catch at all (though depending on the language, you may need a finally block to do function cleanup).  You just let the exception wind up the stack.  So your function either succeeds or passes an exception.

 

This is all well and good until it isn't all well and good.  If you are writingGUI code for instance you can' just allow the exception to keep going up the stack unless you want your users to stop using your product.

I'm sorry, I wasn't clear.  You should let it go up until you can actually deal with it.  Catching it in the middle, where you don't know what you should because of the exception is bad form.  It's usually the upper levels that know whether it should just be logged, throw up a dialog saying "Sorry Dave, I can't do that." or try an alternate method.

The OP looked like middle level code simply eating the exception and going on with life. 

 

Yes, I was just wanting to point out that sometimes you are writing code that can't pass exceptions to somewhere else.  The general idea of exception is exactly that.  Something to pass along until something knows what to do with it.

 You are stepping into a "holy war" when starting a discussion about "single exit point" vs "multiple exit point" which I believe what your post is really regarding. Personally I go with "single exit point" and only have a return statement at the end of the method.

 Talking specifically about a try/catch block IMHO having a return statement in a catch block shows fundamentally flawed exception handling. As posters above have mentioned if the code knows how to handle the exception then handle it and continue on, if it does not know how to handle it (i.e. what business rules apply to receiving that exception) it should bubble the exception up and eventually that exception would reach something that knows how to handle it. If nothing knows how to handle it then it is more than likely a fatal exception.

mjparme:

You are stepping into a "holy war" when starting a discussion about "single exit point" vs "multiple exit point" which I believe what your post is really regarding. Personally I go with "single exit point" and only have a return statement at the end of the method.

Wow, your code must be an overly-verbose mess of slop from trying to enforce such a useless superstition on yourself. 

mjparme:

Personally I go with "single exit point" and only have a return statement at the end of the method.

Lemme guess, your functions often end like this:

            }
          }
        }
      }
    }
  }
  return;
}

bstorer:

mjparme:

Personally I go with "single exit point" and only have a return statement at the end of the method.

Lemme guess, your functions often end like this:

            }
          }
        }
      }
    }
  }
  return ret;
}

FTFY

morbiuswilters:

mjparme:

You are stepping into a "holy war" when starting a discussion about "single exit point" vs "multiple exit point" which I believe what your post is really regarding. Personally I go with "single exit point" and only have a return statement at the end of the method.

Wow, your code must be an overly-verbose mess of slop from trying to enforce such a useless superstition on yourself. 

 

 

Single exit point is overly-verbose??? Slop? I think you have it backwards, multiple-exit point is far sloppier than single-exit point.

bstorer:

mjparme:

Personally I go with "single exit point" and only have a return statement at the end of the method.

Lemme guess, your functions often end like this:

            }
          }
        }
      }
    }
  }
  return;
}

 

 

Umm, no, not at all. Why would my methods end like that?

mjparme:

Umm, no, not at all. Why would my methods end like that?

 

int myfunction() {
  int r;
  r = doSomething();
  if (r==SUCCESS) {
    r = doSomethingElse();
    if (r!=ERROR) {
      r =doYetAnotherThing();
      if (r==SUCCESS_TOO) {
         ...

instead of

int myfunction() {
  int r;
  r = doSomething();
  if (r!=SUCCESS) {
    return r;
  }

  r = doSomethingElse();
  if (r==ERROR) {
    return r;
  }

  r =doYetAnotherThing();
  if (r!=SUCCESS_TOO) {
    return r;
  }
  ...


mjparme:

bstorer:

mjparme:

Personally I go with "single exit point" and only have a return statement at the end of the method.

Lemme guess, your functions often end like this:

            }
          }
        }
      }
    }
  }
  return;
}

 

 

Umm, no, not at all. Why would my methods end like that?

... when they can end like this?

  the_end:
    return ret;
}

mjparme:

Talking specifically about a try/catch block IMHO having a return statement in a catch block shows fundamentally flawed exception handling. As posters above have mentioned if the code knows how to handle the exception then handle it and continue on, if it does not know how to handle it (i.e. what business rules apply to receiving that exception) it should bubble the exception up and eventually that exception would reach something that knows how to handle it. If nothing knows how to handle it then it is more than likely a fatal exception.

But what if the suggested way of handling the exception is to return some pre-defined default value?

mjparme:

Single exit point is overly-verbose??? Slop? I think you have it backwards, multiple-exit point is far sloppier than single-exit point.

Nonsense. An unnecessary adhesion to the single-exit philosophy results in more nested code, more break and continue statements (unless you consider those to be multiple-exit, too, in which case you've just created even more nesting), and more local variables to hold flags and return values. And how, Mr. Single-Exit-Point-Proponent, do you throw exceptions? Are those not exit points, too? Let me give you a couple pseudocode examples; you tell me which of the following is cleaner and more readable:

int multiple_exit_fib (int n)
    if n < 0
        return ERR
    if n == 0
        return 0
    if n == 1
        return 1
    return multiple_exit_fib(n - 1) + multiple_exit_fib(n - 2)

int single_exit_fib (int n)
    if n < 0
        ret = ERR
    else if n == 0
        ret = 0
    else if n == 1
        ret = 1
    else
        ret = single_exit_fib(n - 1) + single_exit_fib(n - 2)
   return ret

bstorer:

int single_exit_fib (int n)
    if n < 0
        ret = ERR
    else if n == 0
        ret = 0
    else if n == 1
        ret = 1
    else
        ret = single_exit_fib(n - 1) + single_exit_fib(n - 2)
   return ret

This example is much cleaner IMHO. Much rather use the else-if than the ifs with multiple returns. So in addition to being an evil multiple exit point person you are also an evil no bracket around single line control structure person:-)

Yes, exception handling is an exit point although I find that a moot point in any multiple-exit vs single exit point discussion because exception handling code is readily apparent whereas it is quite easy to miss a "return" statement amonst a lot of other code. Break and continue aren't method exit points so I am not sure how those are relevent.

 Plus, exception handling, break, and continue are tired arguments from the multiple exit point camp. Can you come up with something original? :-)

mjparme:

Single exit point is overly-verbose??? Slop? I think you have it backwards, multiple-exit point is far sloppier than single-exit point.

Do you know what overly-verbose means?  It means you spend more time nesting and contorting the code so you can have one exit point.  Multiple exit point is not sloppier and you aren't going to find many decent software engineers who advocate for single exit point.