views:

361

answers:

12
bool fn()
{
  if(something bad happen)
     return false;
  ..
}


void gn()
{
  assert(something == true);
  ..

}

When I write a function in production code, which way should I choose?

+1  A: 

IMHO, assert is to assert the developer in debug environment that something unexpected (wrong) is happening. Not all asserts need to be critical and it is up to the developers to put the asserts for the real failures.

In the release mode you need to return false only when the error is critical and there is no point in continuing the execution.

aJ
A: 

Use the assert when something bad should never happen if the code is working properly. An example:

int div( int a, int b ) {
  assert( b != 0 );
  return a / b;
}

Here, the calling code has the responsibility to make sure that div() is never called with a second parameter of zero. If it does, your code is in an unknown state and should be terminated.

Use if() ... return when an error can occur that can be dealt with. For example, opening a file can fail and should be detected and dealt with by the program - failure does not put the program into an unknown state.

anon
A: 

Simple answer is both, but the second is more important. The assert checks a design assumption, thus before calling fn() I would have pre-conditions, e.g. that a given resource might be available. The assert will typically do nothing in release code, though not necessarily. If it is important that your function, gn, require something be true to work correctly, it eithers uses the if statement and returns and error code, or throws an exception. Assert typically does nothing in release code and thus your function might crash in this situation.

See also the more detailed answers given to this question

Shane MacLaughlin
A: 

That really depends on language/runtime environment, and the nature of the error.

In general, the program cannot recover from assertion failures (i.e. it will crash). That may or may not be what you want.

If your environment supports exceptions, you could also use these, that allows you to handle the error if you want to, and will abort with a helpful message if you dont handle the error.

The "return false" thing is error prone (b/c people forget to check the return value), so I'd use it only if neither assert nor exceptions are available.

sleske
+5  A: 

Assert, at least in .NET, is used for testing. Its specific use is put succinctly here:

An assertion is best used to test a condition only when all of the following hold:

* the condition should never be false if the code is correct,
* the condition is not so trivial so as to obviously be always true, and
* the condition is in some sense internal to a body of software.

In production code, I'd recommend the first method; or a try/catch if 'something bad' is never expected to happen; if it is an exceptional condition.

George Stocker
+2  A: 

Consider even to use an Exception when "some thing bad happened" = something exceptional ...

tanascius
A: 

Depends on what you're checking for. In the first case if the bad thing that happened breaks the contract for your class, throw an exception that will mean something to the client. Returning false won't let any know that the state is incorrect.

Keith Bloom
+3  A: 

Use assert to both document and confirm that program invariants (you know it will always be true) hold. In anything more than a trivial program your understanding of the problem will develop as you write the code, so older code might have a different model, and asserts will help pick this up.

For something that can change (especially outside the programs control) you need to be able to report errors (e.g. invaid parameter values passed to a library function, file does not exist), use the platform/languages preferred mechanism (exceptions, return values, ...).

Richard
Yes it is my point of view. I am using assert() calls to check invariants in my program. When such invariants is not respected, as the assert call generates a coredump for my app (while debugging it), I am sure I won't skip such issue. As well I expect next "developers in charge" to notice something is wrong when the application is crashing down... While a simple return statement could eventually lead to dump some lines in a file, that the developer could avoid. I don't mean you may avoid if() tests. I just mean assert() is a way to ensure devs have been strongly notified there was an issue.
yves Baumes
+1  A: 

Assertions are for quality control purposes only. Removing them must not alter the behaviour of your program (i.e. your program must not rely on assertions to handle anything at all). Use exceptions if something happened that should not happen, but can happen under bad circumstances - e.g. network connection lost. Use return values to indicate success when failure is also an option.

ammoQ
A: 

I would use assert whenever something like a contract is broken. What do I mean by this: in math, division by zero is not defined. So the contract with the division function is broken. More on contract see the progromming language Eiffel.

In some cases I would then implement a second function that has additional parameter(s) with default values and handles the exception, like StrinToIntegerDef(string, default), that returns default when string does not convert to Integer.

Return false can be done in cases where normal execution of the program is not at stake.

I would prefer meaningful exceptions, at least of the form assert(condition, message), if the language allows for that.

Also, if there are several possibilities to fail, return some enum values instead of false.

Ralph Rickenbach
A: 

Use assertions to mandate the function's preconditions (conditions that must hold true if the function is to have any meaning).

Cirno de Bergerac
Also invariants and (at the end of functions) postconditions.
Richard
+1  A: 

Your "return false" is also called a GuardClause -- as explained by Ward Cunningham:

...[G]uards are similar to assertions in that both protect the subsequent code from special cases. Guards differ from assertions in that they make a tangible contribution to the logic of the method and thus cannot be safely omitted as part of an optimization. I borrowed the term guard from EwDijkstra when naming this pattern.

If your guard clause is of any complexity it's often helpful to encapsulate using the BouncerPattern.

As Ward points out, you should use assertions when the code can be safely omitted. If you have an assert keyword in your language, compilers will often strip assertions from production code.

Today we recognize that in most cases it's not a good idea to put assertions in your code. Classic separation of concerns. Separate assertions from the code and encapsulate them in a unit test. That eliminates the complexity of having to recompile your code without assertions to be distributed (you won't distribute your tests...) and also gives you a suite of tests that can be run continuously to catch regressions and drive refactorings.

Other answers have mentioned Design by Contract which factors these concerns completely out of your code and into declared preconditions, postconditions and invariants that are enforced around the code in question. If you do this often, you may consider looking into a Design by Contract framework for your language.

cwash

related questions