views:

532

answers:

9

The code in this question made me think

assert(value>0); //Precondition
if (value>0)
{
  //Doit
}

I never write the if-statement. Asserting is enough/all you can do. "Crash early, crash often"

CodeComplete states:

  • The assert-statement makes the application Correct
  • The if-test makes the application Robust

I don't think you've made an application more robust by correcting invalid input values, or skipping code:

assert(value >= 0 );  //Precondition
assert(value <= 90);  //Precondition
if(value < 0)         //Just in case
  value = 0;
if (value > 90)       //Just in case
  value = 90;
//Doit

These corrections are based on assumptions you made about the outside world. Only the caller knows what "a valid input value" is for your function, and he must check its validity before he calls your function.

To paraphrase CodeComplete: "Real-world programs become too messy when we don't rely solely on assertions."

Question: Am I wrong, stuborn, stupid, too non-defensive...

Thanks in advance!

Jan

+9  A: 

The problem with trusting just Asserts, is that they may be turned off in a production environment. To quote the wikipedia article:

Most languages allow assertions to be enabled or disabled globally, and sometimes independently. Assertions are often enabled during development and disabled during final testing and on release to the customer. Not checking assertions avoiding the cost of evaluating the assertions while, assuming the assertions are free of side effects, still producing the same result under normal conditions. Under abnormal conditions, disabling assertion checking can mean that a program that would have aborted will continue to run. This is sometimes preferable. Wikipedia

So if the correctness of your code relies on the Asserts to be there you may run into serious problems. Sure, if the code worked during testing it should work during production... Now enter the second guy that works on the code and is just going to fix a small problem...

Tnilsson
+5  A: 

I some cases, asserts are disabled when building for release. You may not have control over this (otherwise, you could build with asserts on), so it might be a good idea to do it like this.

The problem with "correcting" the input values is that the caller will not get what they expect, and this can lead to problems or even crashes in wholly different parts of the program, making debugging a nightmare.

I usually throw an exception in the if-statement to take over the role of the assert in case they are disabled

assert(value>0);
if(value<=0) throw new ArgumentOutOfRangeException("value");
//do stuff
Rik
I like throwing the Exception: it always works(production and debug), but why then would one write the assert statement too? That only works "half" of the times.
jan
This is simply a matter of convention, I think. When I see an uncatched exception, millions of thing might have caused it. When I spot a failed assert, I know I screwed up myself and it's easier for me to find the cause.
Rik
+1  A: 

If I remember correctly from CS-class

Preconditions define on what conditions the output of your function is defined. If you make your function handle errorconditions your function is defined for those condition and you don't need the assert statement.

So I agree. Usually you don't need both.

As Rik commented this can cause problems if you remove asserts in released code. Usually I don't do that except in performance-critical places.

Mendelt
The problem for this approach is that a crash resulting from the illegal call might happen in a wholly different part of the program, which makes debugging it a nightmare.
Rik
I agree. I guess my answer was incomplete. I usually don't remove asserts in released code. I'll fix my answer.
Mendelt
A: 

A problem with assertions is that they can (and usually will) be compiled out of the code, so you need to add both walls in case one gets thrown away by the compiler.

Vinko Vrsalovic
+2  A: 

Don't forget that most languages allow you to turn off assertions... Personally, if I was prepared to write if tests to protect against all ranges of invalid input, I wouldn't bother with the assertion in the first place.

If, on the other hand you don't write logic to handle all cases (possibly because it's not sensible to try and continue with invalid input) then I would be using the assertion statement and going for the "fail early" approach.

Andrew
A: 

For internal functions, ones that only you will use, use asserts only. The asserts will help catch bugs during your testing, but won't hamper performance in production.

Check inputs that originate externally with if-conditions. By externally, that's anywhere outside the code that you/your team control and test.

Optionally, you can have both. This would be for external facing functions where integration testing is going to be done before production.

Andrew Johnson
+1  A: 

I would disagree with this statement:

Only the caller knows what "a valid input value" is for your function, and he must check its validity before he calls your function.

Caller might think that he know that input value is correct. Only method author knows how it suppose to work. Programmer's best goal is to make client to fall into "pit of success". You should decide what behavior is more appropriate in given case. In some cases incorrect input values can be forgivable, in other you should throw exception\return error.

As for Asserts, I'd repeat other commenters, assert is a debug time check for code author, not code clients.

aku
A: 

I should have stated I was aware of the fact that asserts (here) dissappear in production code.

If the if-statement actually corrects invalid input data in production code, this means the assert never went off during testing on debug code, this means you wrote code that you never executed.

For me it's an OR situation:

(quote Andrew) "protect against all ranges of invalid input, I wouldn't bother with the assertion in the first place." -> write an if-test.

(quote aku) "incorrect input values can be forgivable" -> write an assert.

I can't stand both...

jan
+2  A: 

Use assertions for validating input you control: private methods and such.

Use if statements for validating input you don't control: public interfaces designed for consumption by the user, user input testing etc.

Test you application with assertions built in. Then deploy without the assertions.

Daren Thomas