Defensive Programming - Where do you Draw the Line?

[Smithers]

Please consider this humble method:

public void ResetCounters()
{
m_TotalExceptionsDetected = 0;
m_TotalMessagesSent = 0;
}

Given no further information, would you wrap those two lines in a try...
catch block?

The above is a specific example of a more general question... when an
exception is incredibly unlikely to occur, is there any good reason to add
error handling, assertions, and other validation code?

The bigger picture is this: I'm rewriting a few utilities that I wrote a few
years ago. They have been humming along just fine in production. As part of
my current refactoring-from-1.1-to3.5 effort... in addition to adding a few
new capabilities, I'm wanting to make things more "bullet proof" - not out
of necessity (such tasks would have been done by now), but more for the sake
of doing it "right" or at least better. So I look at the above and think
that it's so incredibly unlikely that anything would ever go wrong with
that - so is there any reason to add the extra code - little as it may be -
for a try... catch block.

And yes, I know "it all depends" [on usage context, etc] - but I'd
appreciate your additional thoughts beyond that.

Do any of you actually put at least one try... catch block in *every* method
you ever write? If not, when do you NOT put one in for reasons other than
laziness?

Thanks.

 

[Jon Skeet [C# MVP]]

Please consider this humble method:

public void ResetCounters()
{
m_TotalExceptionsDetected = 0;
m_TotalMessagesSent = 0;
}

Given no further information, would you wrap those two lines in a try...
catch block?

The above is a specific example of a more general question... when an
exception is incredibly unlikely to occur, is there any good reason to add
error handling, assertions, and other validation code?

I write very few try/catch blocks. I write far more try/finally blocks,
usually implicitly with "using" statements. Well-written error-handling
code *tends* to be at a very high level, indicating that something
within a potentially very deep stack has failed. There's often no need
to handle the error other than at the top level.

The bigger picture is this: I'm rewriting a few utilities that I wrote a few
years ago. They have been humming along just fine in production. As part of
my current refactoring-from-1.1-to3.5 effort... in addition to adding a few
new capabilities, I'm wanting to make things more "bullet proof" - not out
of necessity (such tasks would have been done by now), but more for the sake
of doing it "right" or at least better. So I look at the above and think
that it's so incredibly unlikely that anything would ever go wrong with
that - so is there any reason to add the extra code - little as it may be -
for a try... catch block.

What would you even do in the catch block? You can't really recover
from the error. The best you could do is log and then rethrow - but in
that case, you might as well catch at the top level and log there.

Do any of you actually put at least one try... catch block in *every* method
you ever write? If not, when do you NOT put one in for reasons other than
laziness?

It should be the other way round - if your method fails to execute,
what good reason do you have for *not* just letting the exception
bubble up? How are you going to add value to that error-reporting
mechanism? Can you recover from the error?

 

[Chris Mullins [MVP - C#]]

Please consider this humble method:

public void ResetCounters()
{
m_TotalExceptionsDetected = 0;
m_TotalMessagesSent = 0;
}

Given no further information, would you wrap those two lines in a try...
catch block?

I would continue allowing this to be a humble method, just as it's written.

As Jon said, even if you put in a try/catch, what would you do here? There's
just no point.

The whole point of Exception handling is that every method doesn't have a
try/catch. If it did, it would be no better than (or even worse than) "On
Error Resume Next" or some other horrible error handling mechanism.

Do any of you actually put at least one try... catch block in *every*
method you ever write?

Hell no! If I saw code that did this, I would spend the rest of the day
"educating" the original developer, and then stand over his shoulder all
week long as he rips it all out.

If not, when do you NOT put one in for reasons other than laziness?

In general:
- put try/catch blocks at high levels in your code, where they can actually
do something. Sometimes "something" is defined as "log the error + [Continue
| Exit | Alert]."
- put try/catch blocks at places in your code where you can actually handle
the error.

 

[Peter Duniho]

Please consider this humble method:

public void ResetCounters()
{
m_TotalExceptionsDetected = 0;
m_TotalMessagesSent = 0;
}

Given no further information, would you wrap those two lines in a try...
catch block?

I agree with Jon's reply. Don't catch an exception unless either a) you
have some cleanup you need to do, or b) you can actually gracefully
recover from the exception.

This means that most code doesn't have any exception handling.

In your specific example, it's not clear what those identifiers are.
However, the naming convention is one usually reserved for fields. It's
not clear to me how the code could throw an exception anyway, but
perhaps you've just left out some specific information about the
identifiers (maybe they are properties that do something more than just
setting a value type?).

However, even if the assignments could throw exceptions, the question is
begged: what would you do in the exception handler? How could you
gracefully recover from an error? The caller certainly will assume that
the counters have been successfully reset when the function returned; if
you catch the exception and return normally, you've now put your program
into some unexpected, if not completely invalid, state.

The above is a specific example of a more general question... when an
exception is incredibly unlikely to occur, is there any good reason to add
error handling, assertions, and other validation code?

Whether to write an exception handler depends on whether you can or must
do something useful in handling the exception, not with respect to how
likely exception is.

The bigger picture is this: I'm rewriting a few utilities that I wrote a few
years ago. They have been humming along just fine in production. As part of
my current refactoring-from-1.1-to3.5 effort... in addition to adding a few
new capabilities, I'm wanting to make things more "bullet proof" - not out
of necessity (such tasks would have been done by now), but more for the sake
of doing it "right" or at least better.

"Bullet-proofing" is a fine goal. However, you have to be sure that you
are really bullet-proofing. Sweeping errors under the rug by writing
exception handling that doesn't actually do anything doesn't
bullet-proof anything; it just defers the problem to some later,
more-difficult-to-analyze point.

So I look at the above and think
that it's so incredibly unlikely that anything would ever go wrong with
that - so is there any reason to add the extra code - little as it may be -
for a try... catch block.

Well, again...in that specific example, how could an exception occur?

And yes, I know "it all depends" [on usage context, etc] - but I'd
appreciate your additional thoughts beyond that.

Do any of you actually put at least one try... catch block in *every* method
you ever write? If not, when do you NOT put one in for reasons other than
laziness?

I definitely do _not_ put at least one try/catch block in every method.
The vast majority of methods in my code do not include any exception
handling at all. In my own code, the only place you'll ever see an
exception handling block is where I can actually respond to the
exception in a useful way. Sometimes this is just a matter of cleaning
up and letting the exception continue up the call stack (using or
try/finally) and sometimes this is a matter of detecting and recovering
from an error.

Pete

 

[John Duval]

Please consider this humble method:

public void ResetCounters()
{
m_TotalExceptionsDetected = 0;
m_TotalMessagesSent = 0;

}

Given no further information, would you wrap those two lines in a try...
catch block?

The above is a specific example of a more general question... when an
exception is incredibly unlikely to occur, is there any good reason to add
error handling, assertions, and other validation code?

The bigger picture is this: I'm rewriting a few utilities that I wrote a few
years ago. They have been humming along just fine in production. As part of
my current refactoring-from-1.1-to3.5 effort... in addition to adding a few
new capabilities, I'm wanting to make things more "bullet proof" - not out
of necessity (such tasks would have been done by now), but more for the sake
of doing it "right" or at least better. So I look at the above and think
that it's so incredibly unlikely that anything would ever go wrong with
that - so is there any reason to add the extra code - little as it may be -
for a try... catch block.

And yes, I know "it all depends" [on usage context, etc] - but I'd
appreciate your additional thoughts beyond that.

Do any of you actually put at least one try... catch block in *every* method
you ever write? If not, when do you NOT put one in for reasons other than
laziness?

Thanks.

Hi Smithers,
No I would not put a try block in that routine. I don't see the point
to it (what would you do in a catch or finally block?). Basically,
put a try block where you need to do something in either the finally
or catch blocks. If you can't think of code you would put in a
finally or catch block (I can't for your sample code) then that's a
good hint that you don't need it.

My feeling is that you can make your code more bulletproof by
liberally adding check code that *throws* more exceptions (or does
more debug Asserts), which will help you identify situations where the
data isn't what you'd expect (making sure objects are not null,
verifying values are within range, etc...). The further upstream you
find the error, the easier it is to debug your problem. I've seen a
lot of code that just exits the method if an unexpected condition is
encountered, but that just sweeps the problem further downstream.

Also I agree with Jon's comment that exceptions in general should be
allowed to bubble up to a top-level (or higher-level) exception
handler... it's usually where you have the most context to do
something anyways (show message to user, etc...)
John

 

[sloan]

I concur

try
{

}
finally
{

}

blocks should exist in your code ALOT more than
try/catch or try/catch/finally blocks.



Brad Abrams talked about this as well when he visited our UserGroup

http://msdn2.microsoft.com/en-us/library/ms229005(VS.80).aspx
//Quote//
Do use try-finally and avoid using try-catch for cleanup code. In
well-written exception code, try-finally is far more common than try-catch.
The purpose of a catch clause is to allow you to handle exceptions (for
example, by logging a non-fatal error). The purpose of a finally clause is
to allow you to execute cleanup code regardless of whether an exception was
thrown. If you allocate expensive or limited resources such as database
connections or streams, you should put the code to release them inside a
finally block.

 

[Chris Mullins [MVP - C#]]

It's funny, but I've always disliked the Try/Finally syntax. I may be the
only one, but... ick.

I find myself writing specialized classes, and then using Using blocks
everywhere. This way I'm exception safe, and I don't end up with 10 or so
extra lines of code in each method.

public class ReaderLock : IDisposable
{
private ReaderWriterLock _rw;
public void ReaderLock (ReaderWriterLock rw) { _rw = rw;
_rw.EnterReadLock();}
public boid Dispose () { rw.ExitReadLock();}
}

Then all my code ends up looking like (4 lines of code):
using (new ReaderLock(collectionRWLock))
{
// ... code goes here
}

instead of (9 lines of code):
collectionWRLock.EnterReadLock();
try
{
// Code Goes here
}
finally
{
collectionRWLock.ExitReadLock();
}


I just don't like all the extra lines of the try/finally, and the visual
verbosity they bring for no real benefit. Especially if you're not doing
anything meaningful in them.

--
Chris Mullins

I concur

try
{

}
finally
{

}

blocks should exist in your code ALOT more than
try/catch or try/catch/finally blocks.



Brad Abrams talked about this as well when he visited our UserGroup

http://msdn2.microsoft.com/en-us/library/ms229005(VS.80).aspx
//Quote//
Do use try-finally and avoid using try-catch for cleanup code. In
well-written exception code, try-finally is far more common than
try-catch.
The purpose of a catch clause is to allow you to handle exceptions (for
example, by logging a non-fatal error). The purpose of a finally clause is
to allow you to execute cleanup code regardless of whether an exception
was thrown. If you allocate expensive or limited resources such as
database connections or streams, you should put the code to release them
inside a finally block.

 

[sloan]

That's true as well.


That I would put under personal preference.


I guess I've just been sticking to what Brad went over moreso than the
"using", even though as I've migrated to 2.0/3.0, I do more using
statements.

It's funny, but I've always disliked the Try/Finally syntax. I may be the
only one, but... ick.

I find myself writing specialized classes, and then using Using blocks
everywhere. This way I'm exception safe, and I don't end up with 10 or so
extra lines of code in each method.

public class ReaderLock : IDisposable
{
private ReaderWriterLock _rw;
public void ReaderLock (ReaderWriterLock rw) { _rw = rw;
_rw.EnterReadLock();}
public boid Dispose () { rw.ExitReadLock();}
}

Then all my code ends up looking like (4 lines of code):
using (new ReaderLock(collectionRWLock))
{
// ... code goes here
}

instead of (9 lines of code):
collectionWRLock.EnterReadLock();
try
{
// Code Goes here
}
finally
{
collectionRWLock.ExitReadLock();
}


I just don't like all the extra lines of the try/finally, and the visual
verbosity they bring for no real benefit. Especially if you're not doing
anything meaningful in them.

 

[Smithers]

The "humble method" in the OP here is setting field values.

The reason for the OP is that I have heard a lot of advice over the years
about what we should or should not do under the guise of "best practice" or
"defensive programming" etc. One of those pieces of advice is "every method
should have a try/catch block". I've never done it, and for reasons similar
to those pointed out already in this thread. So I was just wondering what
some of you think about it.

I cannot provide a source for that particular piece of advice regarding
try/catch. But I have heard it and it has bothered me for a while... causing
me to wonder if I was missing something.Another such piece of advice which I
don't get is from Juval Lowy which states "Assert every assumption. On
average every fifth line is an assertion." That's #14 in his
self-proclaimed defacto C# coding standards list (available at
www.idesign.net).

It seems to me that if we were to follow *every* "best practice" and piece
of advice at the same time, then our code would be at least huge, probably
unreadable, and would possibly break.

To digress just a bit (but for purposes of analogy), I was recently in
Canada and they had a HUGE health food store. It was about 3000 sq ft of
health supplements, remedies, vitamins, minerals, organic this, and organic
that. Pine nut oil. Flax seed extract. Every aisle crammed with stuff I'd
never heard of before. I couldn't help but think that (1) for every single
product in there, somebody swears by it; (2) mankind has gotten along just
fine without all this stuff for all time up 'til now; and (3) if I were to
take one of everything in the store I'd probably die within hours if not
minutes. When I first saw that tiny bottle of pine nut oil I couldn't help
but think about Juval Lowy's advice to test *every* assumption and write an
assertion into every 5th line on average.

-S

Please consider this humble method:

public void ResetCounters()
{
m_TotalExceptionsDetected = 0;
m_TotalMessagesSent = 0;
}

Given no further information, would you wrap those two lines in a try...
catch block?

I agree with Jon's reply. Don't catch an exception unless either a) you
have some cleanup you need to do, or b) you can actually gracefully
recover from the exception.

This means that most code doesn't have any exception handling.

In your specific example, it's not clear what those identifiers are.
However, the naming convention is one usually reserved for fields. It's
not clear to me how the code could throw an exception anyway, but perhaps
you've just left out some specific information about the identifiers
(maybe they are properties that do something more than just setting a
value type?).

However, even if the assignments could throw exceptions, the question is
begged: what would you do in the exception handler? How could you
gracefully recover from an error? The caller certainly will assume that
the counters have been successfully reset when the function returned; if
you catch the exception and return normally, you've now put your program
into some unexpected, if not completely invalid, state.

The above is a specific example of a more general question... when an
exception is incredibly unlikely to occur, is there any good reason to
add error handling, assertions, and other validation code?

Whether to write an exception handler depends on whether you can or must
do something useful in handling the exception, not with respect to how
likely exception is.

The bigger picture is this: I'm rewriting a few utilities that I wrote a
few years ago. They have been humming along just fine in production. As
part of my current refactoring-from-1.1-to3.5 effort... in addition to
adding a few new capabilities, I'm wanting to make things more "bullet
proof" - not out of necessity (such tasks would have been done by now),
but more for the sake of doing it "right" or at least better.

"Bullet-proofing" is a fine goal. However, you have to be sure that you
are really bullet-proofing. Sweeping errors under the rug by writing
exception handling that doesn't actually do anything doesn't bullet-proof
anything; it just defers the problem to some later,
more-difficult-to-analyze point.

So I look at the above and think that it's so incredibly unlikely that
anything would ever go wrong with that - so is there any reason to add
the extra code - little as it may be - for a try... catch block.

Well, again...in that specific example, how could an exception occur?

And yes, I know "it all depends" [on usage context, etc] - but I'd
appreciate your additional thoughts beyond that.

Do any of you actually put at least one try... catch block in *every*
method you ever write? If not, when do you NOT put one in for reasons
other than laziness?

I definitely do _not_ put at least one try/catch block in every method.
The vast majority of methods in my code do not include any exception
handling at all. In my own code, the only place you'll ever see an
exception handling block is where I can actually respond to the exception
in a useful way. Sometimes this is just a matter of cleaning up and
letting the exception continue up the call stack (using or try/finally)
and sometimes this is a matter of detecting and recovering from an error.

Pete

 

[Jon Skeet [C# MVP]]

It's funny, but I've always disliked the Try/Finally syntax. I may be the
only one, but... ick.

I don't mind the syntax for try/finally when "using" doesn't do the
job, but when "using" is available it's clearly neater. I very rarely
write "manual" finally clauses in C#.

I *do* wish that you could add a "catch" clause to "using" though:

using (...)
{
}
catch (WebException we)
{
....
}

could convert to:

try (...)
{
}
catch (WebException we)
{
....
}
finally
{
// Dispose
}

When you *do* want to catch exceptions in addition to using a "using"
statement, the extra level of nesting reduces the readability somewhat.

IMO, anyway...

 

[=?ISO-8859-1?Q?Arne_Vajh=F8j?=]

I write very few try/catch blocks. I write far more try/finally blocks,
usually implicitly with "using" statements. Well-written error-handling
code *tends* to be at a very high level, indicating that something
within a potentially very deep stack has failed. There's often no need
to handle the error other than at the top level.

I agree.

One of the biggest (maybe the biggest) advantage of the exception
concept is the ability to automatically send the info up the call
stack without a zillion tests on return values.

If exceptions were always to be caught where they happened we
could (almost) as well use return values. Exceptions are intended
to be caught high up in the call stack where there may be code that
has a chance of doing something reasonable to handle the exception.

I have seen application designs where there were an explicit
design rule that the N top layers was required to catch
exceptions and the M bottom layers was forbidden to catch exceptions
(layers is here in the high specific - low general sense).

Of course there are cases where the rule is not optimal, but it is
a consistent approach that a developer team can understand.

Arne

 

[Ben Voigt [C++ MVP]]

The "humble method" in the OP here is setting field values.

The reason for the OP is that I have heard a lot of advice over the years
about what we should or should not do under the guise of "best practice"
or "defensive programming" etc. One of those pieces of advice is "every
method should have a try/catch block". I've never done it, and for reasons
similar

This is absolutely wrong. Every return value should be checked and error
propagated. Exceptions are an automated way of doing that (with additional
efficiency improvements using long jumps internally instead of masses of
conditionals and return statements). Exceptions are designed to solve the
same problem as return error codes, but they must not be treated as error
codes.

Any well-designed program uses a mix of exceptions and failure codes. (A
failure isn't an error if it is anticipated). For example, it is
appropriate to have List.Find return a failure code instead of an exception,
because calling List.Contains first to avoid an exception would require
iterating the List twice.