views:

1520

answers:

13

Hi all,

There's a discussion going on over at comp.lang.c++.moderated about whether or not assertions, which in C++ only exist in debug builds by default, should be kept in production code or not.

Obviously, each project is unique, so my question here is not so much whether assertions should be kept, but in which cases this is recommendable/not a good idea.

By assertion, I mean:

  • A run-time check that tests a condition which, when false, reveals a bug in the software.
  • A mechanism by which the program is halted (maybe after really minimal clean-up work).

I'm not necessarily talking about C or C++.

My own opinion is that if you're the programmer, but don't own the data (which is the case with most commercial desktop applications), you should keep them on, because a failing asssertion shows a bug, and you should not go on with a bug, with the risk of corrupting the user's data. This forces you to test strongly before you ship, and makes bugs more visible, thus easier to spot and fix.

What's your opinion/experience?

Cheers,

Carl

See related question here


Responses and Updates

Hey Graham,

An assertion is error, pure and simple and therefore should be handled like one. Since an error should be handled in release mode then you don't really need assertions.

That's why I prefer the word "bug" when talking about assertions. It makes things much clearer. To me, the word "error" is too vague. A missing file is an error, not a bug, and the program should deal with it. Trying to dereference a null pointer is a bug, and the program should acknowledge that something smells like bad cheese.

Hence, you should test the pointer with an assertion, but the presence of the file with normal error-handling code.


Slight off-topic, but an important point in the discussion.

As a heads-up, if your assertions break into the debugger when they fail, why not. But there are plenty of reasons a file could not exist that are completely outside of the control of your code: read/write rights, disk full, USB device unplugged, etc. Since you don't have control over it, I feel assertions are not the right way to deal with that.

Carl


Thomas,

Yes, I have Code Complete, and must say I strongly disagree with that particular advice.

Say your custom memory allocator screws up, and zeroes a chunk of memory that is still used by some other object. I happens to zero a pointer that this object dereferences regularly, and one of the invariants is that this pointer is never null, and you have a couple of assertions to make sure it stays that way. What do you do if the pointer suddenly is null. You just if() around it, hoping that it works?

Remember, we're talking about product code here, so there's no breaking into the debugger and inspecting the local state. This is a real bug on the user's machine.

Carl

+1  A: 

I never keep them on in performance critical code.

staffan
+1  A: 

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.

Anders Sandvig
A: 

An assertion is error, pure and simple and therefore should be handled like one.

Since an error should be handled in release mode then you don't really need assertions.

The main benefit I see for assertions is a conditional break - they are much easier to setup than drilling through VC's windows to setup something that takes 1 line of code.

graham.reeds
A: 

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.

roo
A: 

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.

Nicolas
+1  A: 

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.

Steve M
+3  A: 

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.

bruceatk
+13  A: 

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.

Thomas Owens
+1  A: 

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.

Graeme Perrow
A: 

@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.

Thomas Owens
+2  A: 

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.

Qwertie
+4  A: 

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.

MSalters
A: 

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

David Cary