Exception Handling Mistakes: Finally Block Does Not Require The Catch Block

csharp, code 0 comments suggest edit

While reviewing some code this weekend, I had the thought to do a search for the following string throughout the codebase, “catch(Exception” (using the regular expression search of course it looked more like “catch\s(\sException\s*)”.

My intent was to take a look to see how badly Catch(Exception...) was being abused or if it was being used correctly. One interesting pattern I noticed frequently was the following snippet…

try
{
    fs = new FileStream(filename, FileMode.Create);

    //Do Something
}
catch(Exception ex)
{
    throw ex;
}
finally
{
    if(fs != null)
        fs.Close();
}

My guess is that the developer who wrote this didn’t realize that you don’t need a catch block in order to use a finally block. The finally block will ALWAYS fire whether or not there is an exception block. Also, this code is resetting the callstack on the exception as I’ve written about before.

This really just should be.

try
{
    fs = new FileStream(filename, FileMode.Create);

    //Do Something
}
finally
{
    if(fs != null)
        fs.Close();
}

Another common mistake I found is demonstrated by the following code snippet.

try
{
    //Do Something.
}
catch(Exception e)
{
    throw e;
}

This is another example where the author of the code is losing stack trace information. Even worse, there is no reason to even perform a try catch, since all that the developer is doing is rethrowing the exact exception being caught. I ended up removing the try/catch blocks everywhere I found this pattern.

Found a typo or error? Suggest an edit! If accepted, your contribution is listed automatically here.

Comments

avatar

10 responses

  1. Avatar for Scott Williams
    Scott Williams January 9th, 2006

    Exceptions are silly. I just do a try { /* do stuff */ } catch {} and never get any exceptions any more!











    (kidding)

  2. Avatar for Haacked
    Haacked January 9th, 2006

    Wow, I never thought of that! TDD is so tired, Exceptionless programming is so wired! ;)

  3. Avatar for Joe Brinkman
    Joe Brinkman January 9th, 2006

    or better yet how about



    try{

    fs = new FileStream(filename, FileMode.Create);

    //Do Something

    }

    finally

    {

    try{

    fs.Close();

    }

    catch(Exception e){

    throw e;

    }

    }



    Who needs null pointer checks when exceptions are so easy to code for ;)

  4. Avatar for Haacked
    Haacked January 9th, 2006

    Oh geez that hurts my head Joe.

  5. Avatar for jayson knight
    jayson knight January 9th, 2006

    Of course you missed the obvious "using" construct as FileStream derives from IDisposable ;-).



    Things look a little different around here! Either you rethemed, or subText has been updated. First "complaint" (if you can call it that): The text in this textbox for leaving comments is TINY, I'd say about 6pts.



    Lookin' good!

  6. Avatar for jayson knight
    jayson knight January 9th, 2006

    Disregard the "using" portion of my post above, reflecting on FileSystem.Dispose shows that it only flushes the stream, and doesn't close it...I should do my homework first.

  7. Avatar for Haacked
    Haacked January 9th, 2006

    Jayson, no you were right the first time. FileStream.Close simply calls FileStream.Dispose.



    Therefore the using construct would be appropriate and in my opinion even better here. But I was just making the point about how to use try finally.



    I should have probably used an example of a non-disposable type.

  8. Avatar for Binu
    Binu August 30th, 2006

    What if an Exception occurs in a finally block..Don't say we can write try-catch in finally block...any other proper way..plz share me friends ?

  9. Avatar for Abhay Kale
    Abhay Kale February 25th, 2009

    What if there was an exception within the finally block ? How does one handle that ?

  10. Avatar for torzsmokus
    torzsmokus November 23rd, 2011

    Re Abhay: see the StackOverflow question about exceptions in finally blocks and this article about stream handling in java.
    In Java7, there is try-with-resources which is similar to C#’s using.