views:

1723

answers:

11

Hi,

I've just started skimming 'Debugging MS .Net 2.0 Applications' by John Robbins, and have become confused by his evangelism for Debug.Assert(...).

He points out that well-implemented Asserts store the state, somewhat, of an error condition, e.g.:

Debug.Assert(i > 3, "i > 3", "This means I got a bad parameter");

Now, personally, it seems crazy to me that he so loves restating his test without an actual sensible 'business logic' comment, perhaps "i <= 3 must never happen because of the flobittyjam widgitification process".

So, I think I get Asserts as a kind-of low-level "Let's protect my assumptions" kind of thing... assuming that one feels this is a test one only needs to do in debug - i.e. you are protecting yourself against colleague and future programmers, and hoping that they actually test things.

But what I don't get is, he then goes on to say that you should use assertions in addition to normal error handling; now what I envisage is something like this:

Debug.Assert(i > 3, "i must be greater than 3 because of the flibbity widgit status");
if (i <= 3)
{
    throw new ArgumentOutOfRangeException("i", "i must be > 3 because... i=" + i.ToString());
}

What have I gained by the Debug.Assert repetition of the error condition test? I think I'd get it if we were talking about debug-only double-checking of a very important calculation...

double interestAmount = loan.GetInterest();
Debug.Assert(debugInterestDoubleCheck(loan) == interestAmount, "Mismatch on interest calc");

...but I don't get it for parameter tests which are surely worth checking (in both DEBUG and Release builds)... or not. What am I missing?

+1  A: 

IMO it's a loss of development time only. Properly implemented exception gives you a clear picture of what happened. I saw too much applications showing obscure "Assertion failed: i < 10" errors. I see assertion as a temporary solution. In my opinion no assertions should be in a final version of a program. In my practice I used assertions for quick and dirty checks. Final version of the code should take erroneous situation into account and behave accordingly. If something bad happens you have 2 choices: handle it or leave it. Function should throw an exception with meaningful description if wrong parameters passed in. I see no points in duplication of validation logic.

aku
+21  A: 

Assertions are not for parameter checking. Parameter checking should always be done (and precisely according to what pre-conditions are specified in your documentation and/or specification), and the ArgumentOutOfRangeException thrown as necessary.

Assertions are for testing for "impossible" situations, i.e., things that you (in your program logic) assume are true. The assertions are there to tell you if these assumptions are broken for any reason.

Hope this helps!

Chris Jester-Young
Assertions can be used for parameter checking of *internal* method calls (called by code belonging to the same component) methods, as opposed to external method calls (called by another component). For example, I might assert that a private method parameter of type Double isn't a NaN.
RoadWarrior
@Chris: You state that Assertions are not to be used for parameter checking. Is there a reason for this? I tend to throw exceptions for parameter checks especially in constructors when I inject dependent objects. However I'm being told to use Assertions. I don't have a logical explanation to using exceptions rather than assertions. Are you able to clarify? Cheers
Gavin Chin
Don't worry I found out why. According to Jon Skeet, http://stackoverflow.com/questions/1276308/exception-vs-assertion:"Use assertions for internal logic checks within your code, and normal exceptions for error conditions outside your immediate code's control."
Gavin Chin
+2  A: 

I use explicit checks that throw exceptions on public and protected methods and assertions on private methods.

Usually, the explicit checks guard the private methods from seeing incorrect values anyway. So really, the assert is checking for a condition that should be impossible. If an assert does fire, it tells me the there is a defect in the validation logic contained within one of the public routines on the class.

Simon Johnson
A: 

Thansk to all for your thoughts - it seems like I wasn't missing anything, and the feedback (at least so far) suggests that the book I'm reading may be offering some unusual guidance. Maybe it's even wrong ;)

@Simon - I believe I understand your distinction, but would guess that the intention behind what you're saying is the undelying assumption that you have tested your 'user' input in a public method... I'm looking at a routine right now where the inputs are passed to a private method to be parsed. In other words, dangerous values could still exist in private calls. (depending on so many factors etc etc).

Nij
Yes, I meant it as a rule of thumb. Clearly, if you want hand off validation functions to private methods you had better put explicit error checking in them.
Simon Johnson
+9  A: 

There is a communication aspect to asserts vs exception throwing.

Let's say we have a User class with a Name property and a ToString method.

If ToString is implemented like this:

public string ToString()
{
     Debug.Assert(Name != null);
     return Name;
}

It says that Name should never null and there is a bug in the User class if it is.

If ToString is implement like this:

public string ToString()
{
     if ( Name == null )
     {
          throw new InvalidOperationException("Name is null");
     }

     return Name;
}

It says that the caller is using ToString incorrectly if Name is null and should check that before calling.

The implementation with both

public string ToString()
{
     Debug.Assert(Name != null);
     if ( Name == null )
     {
          throw new InvalidOperationException("Name is null");
     }

     return Name;
}

says that if Name is null there bug in the User class, but we want to handle it anyway. (The user doesn't need to check Name before calling.) I think this is the kind of safety Robbins was recommending.

hwiechers
A: 

@hwiechers: I take your point re: different implied meanings to the two forms of raising the issue, however I would argue that the wish to handle the error trumps the flagging of an internal inconsistency, and that only one need be applied.

Additionally, say the issue ever does come up from live, reproducing the error in test now results in a different error response, not the same one as live, which could in turn lead to confusion... though granted probably much less than if no checks had been applied ;)

Nij
+2  A: 

An exception can be caught and swallowed making the error invisible to testing. That can't happen with Debug.Assert.

No one should ever have a catch handler that catches all exceptions, but people do it anyway, and sometimes it is unavoidable. If your code is invoked from COM, the interop layer catches all exceptions and turns them into COM error codes, meaning you won't see your unhandled exceptions. Asserts don't suffer from this.

Also when the exception would be unhandled, a still better practice is to take a mini-dump. One area where VB is more powerful than C# is that you can use an exception filter to snap a mini-dump when the exception is in flight, and leave the rest of the exception handling unchanged. Gregg Miskelly's blog post on exception filter inject provides a useful way to do this from c#.

One other note on assets ... they inteact poorly with Unit testing the error conditions in your code. It is worthwhile to have a wrapper to turn off the assert for your unit tests.

Steve Steiner
A: 

Thanks for your input Steve,

Your comments were thought-provoking - especially in relation to Asserts not being caught by overzealous catch statements. The link provided is a bit beyond me right now - but hopefully I'll be able to start learning WinDbg and SOS in the near future... my PC crashed with a BSOD as I was writing my first resonse here... so I have other things to worry about right now ;)

Nij
A: 

Check this out: http://www.devzest.com/blog/post/DebugAssert-vs-Exception.aspx

A: 

Here is by 2 cents.

I think that the best way is to use both assertions and exceptions. The main differences between the two methods, imho, if that Assert statements can be removed easily from the application text (defines, conditional attributes...), while Exception thrown are dependent (tipically) by a conditional code which is harder to remove (multine section with preprocessor conditionals).

Every application exception shall be handled correctly, while assertions shall be satisfied only during the algorithm developement and testing.

If you pass an null object reference as routine parameter, and you use this value, you get a null pointer exception. Indeed: why you should write an assertion? It's a waste of time in this case. But what about private class members used in class routines? When these value are set somewhere, is better to check with an assertion if a null value is set. That's only because when you use the member, you get a null pointer exception but you don't know how the value was set. This cause a restart of the program breaking on all entry point use to set the private member.

Exception are more usefull, but they can be (imho) very heavy to manage and there is the possibility to use too much exceptions. And they requires additional check, maybe undesired to optimize the code. Personally I use exceptions only whenever the code requires a deep catch control (catch statements are very low in the call stack) or whenever the function parameters are not hardcoded in the code.

Luca
+1  A: 

I've thought about this long and hard when it comes to providing guidance on debug vs. assert with respect to testing concerns.

You should be able to test your class with erroneous input, bad state, invalid order of operations and any other conceivable error condition and an assert should never trip. Each assert is checking something should always be true regardless of the inputs or computations performed.

Good rules of thumb I've arrived at:

  1. Asserts are not a replacement for robust code that functions correctly independent of configuration. They are complementary.

  2. Asserts should never be tripped during a unit test run, even when feeding in invalid values or testing error conditions. The code should handle these conditions without an assert occurring.

  3. If an assert trips (either in a unit test or during testing), the class is bugged.

For all other errors -- typically down to environment (network connection lost) or misuse (caller passed a null value) -- it's much nicer and more understandable to use hard checks & exceptions. If an exception occurs, the caller knows it's likely their fault. If an assert occurs, the caller knows it's likely a bug in the code where the assert is located.

Regarding duplication: I agree. I don't see why you would replicate the validation with a Debug.Assert AND an exception check. Not only does it add some noise to the code and muddy the waters regarding who is at fault, but it a form of repetition.

Mark Simpson