CODEWITHSUNDEEP
c#coding-style
Mitch
Mitch

Reputation: 22311

Is there a reason for using Try/Finally with ExceptionThrown variable over Try/Catch

I was perusing the .Net Reference Source and found this gem in ButtonBase.cs at line 408:

bool exceptionThrown = true;
try
{ 
    OnClick();
    exceptionThrown = false; 
}
finally
{
    if (exceptionThrown) 
    {
        // Cleanup the buttonbase state 
        SetIsPressed(false); 
        ReleaseMouseCapture();
    } 
}

Question being, what would motivate someone to use the exceptionThrown flag over just writing it as 

try 
{
    OnClick();
}
catch
{
    SetIsPressed(false);
    ReleaseMouseCapture();
    throw;
}

Is it just stylistic or is there some side-effect I am missing?

Upvotes: 9

Views: 314

Answers (5)

Erik Funkenbusch
Erik Funkenbusch

Reputation: 93464

The reason for this code is two fold. Yes, a Greg mentions, it does not require rethrowing the exception.. but that's not really the reason.

The real reason is one of semantics. Exceptions should not be used to handle control flow. What this is doing is dealing with the fact that if an exception gets thrown within a button, It can leave the buttons visual state as "pressed". This isn't really "handling" the exception. This is merely correcting a visual problem in the event an exception is thrown.

This code does not care what the exception is, and it doesn't want to catch all exceptions because that's poor practice. Further, the code is not doing anything with the exception.. it's just saying "Hey, if we got to the end of the function, then we're all good. If we didn't, then lets reset the button state just to be sure".

So, this is not real exception handling, as such it doesn't catch an exception. It just notices that an exception was thrown and does some cleanup.

EDIT:

This method might be less controversial, and make a lot more sense if it were simply renamed like this, removing any reference to exceptions:

bool cleanupRequired = true;
try
{ 
    OnClick();
    cleanupRequired = false; 
}
finally
{
    if (cleanupRequired) 
    {
        // Cleanup the buttonbase state 
        SetIsPressed(false); 
        ReleaseMouseCapture();
    } 
 }

EDIT:

To support my comment below, I wrote the following test program to test the scenarios:

static void Main(string[] args)
{
    TimeSpan ts = new TimeSpan();
    TimeSpan ts2 = new TimeSpan();
    TimeSpan ts3 = new TimeSpan();
    TimeSpan ts4 = new TimeSpan();
    TimeSpan ts5 = new TimeSpan();
    TimeSpan ts6 = new TimeSpan();
    TimeSpan ts7 = new TimeSpan();
    TimeSpan ts8 = new TimeSpan();

    Stopwatch sw = new Stopwatch();

    // throw away first run
    for (int i = 0; i < 2; i++)
    {
        sw.Restart();
        try
        {
            throw new NotImplementedException();
        }
        catch
        {
            ts = sw.Elapsed;
        }
        sw.Stop();
        ts2 = sw.Elapsed;

        try
        {
            sw.Restart();
            try
            {
                throw new NotImplementedException();
            }
            finally
            {
                ts3 = sw.Elapsed;
            }
        }
        catch
        {
            ts4 = sw.Elapsed;
        }
        sw.Stop();
        ts5 = sw.Elapsed;

        try
        {
            sw.Restart();
            try
            {
                throw new NotImplementedException();
            }
            catch
            {
                ts6 = sw.Elapsed;
                throw;
            }
        }
        catch
        {
            ts7 = sw.Elapsed;
        }
        sw.Stop();
        ts8 = sw.Elapsed;
    }
    Console.WriteLine(ts);
    Console.WriteLine(ts2);
    Console.WriteLine(ts3);
    Console.WriteLine(ts4);
    Console.WriteLine(ts5);
    Console.WriteLine(ts6);
    Console.WriteLine(ts7);
    Console.WriteLine(ts8);
    Console.ReadLine();
}

And I got the following results (I have split them apart to make them easier to read):

00:00:00.0028424
00:00:00.0028453

00:00:00.0028354
00:00:00.0028401
00:00:00.0028427

00:00:00.0028404
00:00:00.0057907
00:00:00.0057951

The last 3 show that when rethrowing the exception using throw; it does not simply pass the existing exception on, it has to recreate the exception and rethrow it, taking twice as long.

As we can see, there isn't a significant difference between catching an exception and not catching, but using a finally. However, rethrowing the exception is where the cost comes in.

This is run in VS 2012 Update 3.

EDIT:

Timings without the debugger. As you can see, rethrowing is still twice as expensive:

00:00:00.0000149
00:00:00.0000154

00:00:00.0000137
00:00:00.0000140
00:00:00.0000146

00:00:00.0000137
00:00:00.0000248
00:00:00.0000251

Upvotes: 11

Mitch
Mitch

Reputation: 22311

Apparently, there are side effects to the throw; that will still remove stack trace information up through at least .Net 2. I imagine this syntax was used to work around the implementation of throw; on early versions of the framework.

This blog article gives two examples of how throw; is not equivalent to never catching an exception at all.

The question Showing stack trace of exception that is re-thrown, rather than stack trace from throw point gives a scenario whereby re-throwing the exception causes different operation in Visual Studio.

Upvotes: 2

John Saunders
John Saunders

Reputation: 161831

A small note: OnClick is a virtual method. It is expected to be overridden by user-written code, which can throw arbitrary exceptions. The code as written expresses the semantics of "clean up from the mess made by those who overrode this method".

Upvotes: 0

nothrow
nothrow

Reputation: 16178

Well, one reason I can think of is (in future) adding the need for catching specific exception, while maintaining the same cleanup behavior. Consider following examples:

try 
{
    OnClick();
}
catch(System.SomeSpecificException ex)
{
    Handle(ex);
    throw;
    // now, we're missing the cleanup
}
catch
{
    SetIsPressed(false);
    ReleaseMouseCapture();
    throw;
}

vs.

var exceptionThrown = true;
try 
{
    OnClick();
    exceptionThrown = false;
}
catch(System.SomeSpecificException ex)
{
    Handle(ex);
    throw;
    // whoa, I added specific handler, but cleanup is called anyway!
}
finally
{
    if (exceptionThrown) 
    {
        // Cleanup the buttonbase state 
        SetIsPressed(false); 
        ReleaseMouseCapture();
    } 
}

Upvotes: 1

AnotherDeveloperNamedGreg
AnotherDeveloperNamedGreg

Reputation: 567

If you use try, catch like that an exception has to be thrown twice. With try, finally only one exception is thrown so it is a lot more efficient, especially if this code gets called often.

Upvotes: 8

Related Questions