Assert Abuse
I see this all the time:
HRESULT Foo::get_Bar(Bar** ppBar)
{
ASSERT(ppBar != NULL);
*ppBar = m_pBar;
return S_OK;
}
Eh, what's the point of that ASSERT there? Without it, the subsequent assignment to *ppBar will cause an exception which will almost invariably pop you right into the debugger at the exact point of failure. It seems redundant. The only use is that you don't see the need for real error handling, but the dialog that the OS throws up for Access Violation is just too scary for users to see (and somehow, the ASSERT box isn't).
You could say it has some value as self-documenting code. But I think it's already clear that if you wanted to get a Bar object, you had better give get_Bar a place to put it.
Eh, but that's a mild form of abuse; it just bugs me because it's a sign of mechanical thinking, a ritual done without a purpose. But nothing bad happens; at worst you've just added an extra line of clutter. What really sucks is THIS nonsense, also far too common:
HRESULT hr = ReadInitializationFile( );
ASSERT(SUCCEEDED(hr));
Can ReadInitializationFile fail? Sure it can! It could be malformed or nonexistent. What happens if this is the case? Well, some hapless developer gets popped into the debugger, they investigate the problem for the better part of an hour, and at the end of it, they make no change to the code -- there's no bug there, just a malformed INI file. They learn to shrug their shoulders and hit F5 or whatever key it is to continue execution. Five minutes later, BAM!, there goes another assertion ... no worry, it's just that bogus one again. Ignore it ...
Get it? You've done your part to train a whole generation of programmers to ignore assertions. Worse yet, you're not even trying to handle the failure in a graceful way.
Just for review: an assert should always indicate a programming error. Assert is not a logging facility. If what you need is a logging facility, there are far better alternatives. An assert should not be thrown for conditions which, however unusual or exceptional, your program handles correctly (or should handle correctly). Instead, an assert should always be a precondition, postcondition, or invariant of the code which should always be true, no matter what data is thrown at it for input, or whatever unusual (valid) code paths taken to get there. For example:
// Finds all the prime factors for the given input. Assumes
// the input has already been error-checked (ie, it's greater than one).
vector<int> FindPrimeFactors(int n)
{
assert(n > 1);
vector<int> ans;
// Many lines of code later ...
// Verify that the factors multiply up to the original value.
assert(accumulate(ans.begin( ), ans.end( ), 1, multiplies<int>( )) == n);
return ans;
}
