Assert is evil
Christopher Baus has been away and whilst he was a away his HTTP proxy shutdown due to a bug which caused an assert and his site was unavailable.
Personally I abhor assert; but I completely agree with what Chris is talking about.
This CodeProject link explains what most people mean by assert. My problem with this view of the world is that, in the majority of cases, these tests go away in production code. In my opinion that’s simply madness; and before you tell me that they have to be compiled out of release builds for performance reasons I’d like to see the reports from all the profiling that you did to come to that conclusion.
Assertions are often used for function argument validation and for object/system state validation. You’ve been passed x, is the caller conforming to your expectations of what x should be able to do? The system is about to do y and it can only do it when x and z are true, are they? Etc.
It seems to me that often asserts are used to patch over poor design. Often they’re thrown into a system haphazardly, much like debug trace statements; we’re debugging y because it’s failing due to x being wrong, let’s assert that x is how we want it in these places so we can work out where it’s wrong… Or, this pointer that we’re passed can never be null so let’s assert it isn’t… Or, this string should only ever contain numeric characters; let’s assert that it does… Etc.
My first problem is that these checks, if they’re important, should be just as important in release code. Even if you have 100% code coverage from your unit tests and the tests actually test 100% of the possible permutations of how your code can be executed you still want to know why that pointer is null if it is null in your release build on one Sunday evening, in July. In fact you’re more likely to want to know as much detail as you can get if the “assertion” fails when the code is live… So, to me, assertions that compile away to nothing in release builds are a waste of time; they’re premature optimisation of the worst kind.
My second problem is that use of asserts tends to encourage poor design; rather than designing an object that can’t be put into inappropriate states the designer simply adds asserts to detect the inappropriate states during debugging. In these cases you don’t need asserts, you need a better design. Likewise with parameter validation, if you’re using C++ then there’s no excuse for a function that takes a pointer and then immediately asserts that the pointer is not null; pass a reference! In C if it’s an error to pass a null pointer then it’s always an error and decent error handling is better all the time is better than code that dissolves away in release builds.
My third problem is that assertions are often hard to test for; try and write a unit test that proves that a “traditional” assert works… It’s complicated because assert ends the program…
So, “traditional” asserts are evil, but what Chris’s system does is good. If your internal validation code fails then pretty much all you can do is shut down. It may be appropriate to try and produce a lot of information about why you’re shutting down, it may not be. In C++ I find that exceptions are very useful here. Most of the time you will want to try to shut down cleanly, if you can. I agree that there are situations where attempting a clean shutdown is worse than just falling over at the point of failure but, in my opinion, they’re rarer than some people would have you believe.
Exceptions can be used in almost all places where, after designing your way out of many of the problems, a traditional assert might still seem appropriate. Using exceptions like this means that all return values are checked, always, but that you, the programmer, don’t usually need to worry about the stuff that “can’t fail”. Code like this:
void CEvent::Set()
{
if (!::SetEvent(m_hEvent))
{
throw CWin32Exception(_T("CEvent::Set()"), ::GetLastError());
}
}
means that failure of SetEvent() is one less thing to worry about. It’s not going to fail and there’s nothing you can do if it does but you DO need to know about it if it DOES fail because it probably means something is seriously wrong somewhere.
[Updated 3rd October]
See the following follow up postings:
