Exception Handling Code

  • Thread starter Thread starter C# Learner
  • Start date Start date
C

C# Learner

In the following two code blocks, DoSomethingUseful throws AnException,
while neither of the other called methods throw any exceptions. Which of
the code blocks is better (in terms of readability, maintainability, etc.),
in your opinions?

A
{
Start();
try
{
DoSomethingUseful();
}
catch (AnException e)
{
throw e;
}
Finish();
}

B
{
try
{
Start();
DoSomethingUseful();
Finish();
}
catch (AnException e)
{
throw e;
}
}
 
C# Learner said:
In the following two code blocks, DoSomethingUseful throws AnException,
while neither of the other called methods throw any exceptions. Which of
the code blocks is better (in terms of readability, maintainability, etc.),
in your opinions?

A
{
Start();
try
{
DoSomethingUseful();
}
catch (AnException e)
{
throw e;
}
Finish();
}

B
{
try
{
Start();
DoSomethingUseful();
Finish();
}
catch (AnException e)
{
throw e;
}
}
Click to expand...

I will use B, I will use A if I want to determine which function will
throw exception in debugging.
 
Hi Learner. I think both are bad.
I would go for this one:

Start();
try
{
DoSomeThingUseful();
}
finally
{
Finnish();
}

And Finnish must know what and how much it should clean up depending on how
far DoSomeThingUseful got.

- Michael S
 
Michael S said:
Hi Learner. I think both are bad.
I would go for this one:

Start();
try
{
DoSomeThingUseful();
}
finally
{
Finnish();
}

And Finnish must know what and how much it should clean up depending
on how far DoSomeThingUseful got.
Click to expand...

You're changing the semantics though - you're calling Finish() when an
exception is being thrown, which the original code didn't. I don't see
anything in the OP's post which suggests that's the desired behaviour.

Personally I'd prefer:

Start();
DoSomethingUseful();
Finish();

The only tiny difference here is what the stack trace will show when an
exception is thrown by DoSomethingUseful() - but I *suspect* the OP
doesn't care about that.
 
Jon's answer is the best but does not explain the reason.

First , you should almost never use...
try { }
catch(Exception e)
{
throw e;
}
The reason is that the "throw" causes the stack trace to get set to the line
of code where the throw statement occurs. This is almost never what you
really want as it discards the original stack trace information and replaces
it with a stack trace that begins on the line of the throw statement.
Instead, you can use
....
catch(Exception e)
{
throw; // NOTE: A bug in v1.1 framework also resets the stack trace
}

or wrap the original exception within a new one and chain the exceptions
together, as in...

try { }
catch(Exception e)
{
throw new Exception("Your message here.",e);
}

I prefer the catch-wrap-throw form. The only justification I can think of
for using 'throw e;' is if you deliberately want to hide information,
perhaps for security reasons. Otherwise you are discarding useful
information.


Next reason is that your code as shown does not perform any useful actions
in the catch statement. If all it is doing is rethrowing the original
exception then adding the catch block adds absolutely nothing of value. If
you are doing some processing in the catch block then of course that changes
things.

Finally, the execution flow under normal conditions is...

Start()
DoSomethingUseful()
Finish()

but if the method DoSomethingUseful() throws an exception then the routine
Finish() will never get called - the flow is then...

Start
DoSomethingUseful()
(SomeHandler up the callstack executes)

You must decide if this is the correct behavior - if so, then you will get
the same results using either A or B. If you want Finish to always execute
then you should put that in the finally block of the try statement.

It really boils down to determining what the desired behavior of the system
should be - it's hard to tell from the information you provided.
 
David Levine said:
First , you should almost never use...
try { }
catch(Exception e)
{
throw e;
}
Click to expand...

I've never heard of this before. Where could I find more info on this?
Instead, you can use
...
catch(Exception e)
{
throw; // NOTE: A bug in v1.1 framework also resets the stack trace
}
Click to expand...

Out of interest, how does

catch (Exception)
{
throw;
}

change matters, in this case?
It really boils down to determining what the desired behavior of the system
should be - it's hard to tell from the information you provided.
Click to expand...

It's as specified -- Start gets called, and then DoSomethingUseful. Then
if DoSomethingUseful doesn't throw an exception, Finish gets called. Maybe
I should've chosen better method names in this example.

Note that this is purely a question of style. I'm wondering whether people
tend to wrap more code than is required, in a 'try' block, or all that is
require and no more.
 
C# Learner said:
I've never heard of this before. Where could I find more info on this?
Click to expand...

I believe Jeffrey Richter's book talks about this. This link has a great
deal of info...

http://blogs.msdn.com/cbrumme/archive/2003/10/01/51524.aspx
Out of interest, how does

catch (Exception)
{
throw;
}

change matters, in this case?
Click to expand...

The throw statement is defined to capture the stack if there is an exception
object argument with the throw statement. If you do not specify an exception
object (i.e. throw e; versus throw;) then it is supposed to rethrow the
original exception without modifying the stack trace.
It's as specified -- Start gets called, and then DoSomethingUseful. Then
if DoSomethingUseful doesn't throw an exception, Finish gets called.
Maybe
I should've chosen better method names in this example.

Note that this is purely a question of style. I'm wondering whether
people
tend to wrap more code than is required, in a 'try' block, or all that is
require and no more.
Click to expand...

I think it's more readable to group as many lines of code together within
the same braces, but that may imply the operations are related when they may
not be. I like to think in terms of intent. Image that you are writing the
code so that 2 years from now, when someone else is maintaining it, you
leave enough breadcrumbs behind to help them fix bugs, modify it. or just
understand it. In the end there are no hard rules about style, just a
collection of best practices that people gravitate to.
 
Back
Top