When should assertions stay in production code?

Allow me to quote Steve McConnell's Code Complete. The section on Assertions is 8.2.

Normally, you don't want users to see assertion messages in production code; assertions are primarily for use during development and maintenance. Assertions are normally compiled into the code at development time and compiled out of the code for production.

However, later in the same section, this advice is given:

For highly robust code, assert and then handle the error anyway.

I think that as long as performance is not an issue, leave the assertion in, but rather than display a message, have it write to a log file. I think that advice is also in Code Complete, but I'm not finding it right now.

Assertions are comments that do not become outdated. They document which theoretical states are intended, and which states should not occur. If code is changed so states allowed change, the developer is soon informed and needs to update the assertion.

Leave assertions turned on in production code, unless you have measured that the program runs significantly faster with them turned off.

"if it's not worth measuring to prove it's more efficient, then it's not worth sacrificing clarity for a performance gamble." -- SteveMcConnell 1993

http://c2.com/cgi/wiki?ShipWithAssertionsOn

In my C++ I define REQUIRE(x) which is like assert(x) except that it throws an exception if the assertion fails in a release build.

Since a failed assertion indicates a bug, it should be treated seriously even in a Release build. When my code's performance matters, I will often use REQUIRE() for higher-level code and assert() for lower-level code that must run fast. I also use REQUIRE instead of assert if the failure condition may be caused by data passed in from code written by a third party, or by file corruption (optimally I would design the code specifically to be well behaved in case of file corruption, but we don't always have time to do that.)

They say you shouldn't show those assert messages to end-users because they won't understand them. So? End users may send you an email with a screen shot or some text of the error message, which helps you debug. If the user simply says "it crashed", you have less ability to fix it. It would be better to send the assertion-failure messages to yourself automatically over the internet, but that only works if the user has internet access and you can get their permission.

Unless profiling shows that the assertions are causing performance problems, I say they should stay in the production release as well.

However, I think this also requires that you handle assertion failures somewhat gracefully. For example, they should result in a general type of dialog with the option of (automatically) reporting the issue to the developers, and not just quit or crash the program. Also, you should be careful not to use assertions for conditions that you actually do allow, but possibly don't like or consider unwanted. Those conditions should be handled by other parts of the code.

If you want to keep them replace them with error handling. Nothing worse than a program just disappearing. I see nothing wrong with treating certain errors as serious bugs, but they should be directed to a section of your program that is equipped to deal with them by collecting data, logging it, and informing the user that your app has had some unwanted condition and is exiting.

Our database server software contains both production and debug assertions. Debug assertions are just that -- they are removed in production code. Production assertions only happen if (a) some condition exists that should never exist and (b) it is not possible to reliably recover from this condition. A production assertion indicates that a bug in the software has been encountered or some kind of data corruption has occurred.

Since this is a database system and we are storing potentially enterprise-critical data, we do whatever we can to avoid corrupted data. If a condition exists that may cause us to store incorrect data, we immediately assert, rollback all transactions, and stop the server.

Having said that, we also try to avoid production assertions in performance-critical routines.

If you're even thinking of leaving assertions on in production, you're probably thinking about them wrong. The whole point of assertions is that you can turn them off in production, because they are not a part of your solution. They are a development tool, used to verify that your assumptions are correct. But the time you go into production, you should already have confidence in your assumptions.

That said, there is one case where I will turn assertions on in production: If we encounter a reproducible bug in production that we're having a hard time reproducing in a test environment, it may be helpful to reproduce the bug with assertions turned on in production, to see if they provide useful information.

A more interesting question is this: In your testing phase, when do you turn assertions off?

I rarely use assertions for anything other that compile time type checking. I would use an exception instead of an assertion just because most languages are built to handle them.

I offer an example

file = create-some-file();
_throwExceptionIf( file.exists() == false, "FILE DOES NOT EXIST");

against

file = create-some-file();
ASSERT(file.exists());

How would the application handle the assertion? I prefer the old try catch method of dealing with fatal errors.

Most of the time, when i use assertion in java (the assert keyword) I automatically add some production codes after. According to the case, it can be a logging message, an exception... or nothing.

According to me, all your assertions are critical in dev release, not in production relase. Some of them must be kept, other must be discarded.

Provided they are handled just as any other error, I don't see a problem with it. Do bear in mind though that failed assertions in C, as with other languages, will just exit the program, and this isn't usually sufficient for production systems.

There are some exceptions - PHP, for instance, allows you to create a custom handler for assertion failures so that you can display custom errors, do detailed logging, etc. instead of just exiting.

@Carl Seleborf

First of all, I'm hoping that this kind of problem would have been caught in testing, but let's say it wasn't for whatever reason. The second advice works - use the assertion to log it and create an error state (throw an exception, return an error value, close the application). In fact, that truly sounds like an exceptional case. But it's been over a year since I've done any work with pointers and memory management, so take my thoughts with a grain of salt.

Of course, you might be right and the second piece of advice should be ignored and the first one followed.

I see asserts as in-line unit tests. Useful for a quick test while developing, but ultimately those assertions should be refactored out to be tested externally in unit tests.

ASSERTIONS are not errors and should not be handled as errors. When an assertion is thrown, this means that there is a bug in your code or alternatively in the code calling your code.

There are a few points to avoid enabling assertions in production code: 1. You don't want your end user to see a message like "ASSERTION failed MyPrivateClass.cpp line 147. The end user is NOT you QA engineer. 2. ASSERTION might influence performance

However, there is one strong reason to leave assertions: ASSERTION might influence performance and timing, and sadly this sometimes matter (especially in embedded systems).

I tend to vote for leaving the assertion on in production code but making sure that these assertions print outs are not exposed to end user.

~Yitzik

I find it best to handle all errors that are in scope, and use assertions for assumptions that we're asserting ARE true.

i.e., if your program is opening/reading/closing a file, then not being able to open the file is in scope -- it's a real possibility, which would be negligent to ignore, in other words. So, that should have error-checking code associated with it.

However, let's say your fopen() is documented as always returning a valid, open file handle. You open the file, and pass it to your readfile() function.

That readfile function, in this context, and probably according to its design specification, can pretty much assume it's going to get a valid file ptr. So, it would be wasteful to add error-handling code for the negative case, in such a simple program. However, it should at least document the assumption, somehow -- ensure somehow --- that this is actually the case, before continuing its execution. It should not ACTUALLY assume that will always be valid, in case it's called incorrectly, or it's copy/pasted into some other program, for example.

So, readfile() { assert(fptr != NULL); .. } is appropriate in this case, whilst full-blown error handling is not (ignoring the fact that actually reading the file would require some error handling system anyway).

And yes, those assertions should stay in production code, unless its absolutely necessary to disable them. Even then, you should probably disable them only within performance-critical sections.

I never keep them on in performance critical code.