Wednesday 28 July 2004 — This is more than 20 years old. Be careful.
I don’t know about you, but I have a hard time reading assert expressions like this:
assert( id->locktype != NO_LOCK || locktype == SHARED_LOCK );
assert( locktype != RESERVED_LOCK || id->locktype == SHARED_LOCK );
The combination of the not-equal and the logical or is difficult to parse out into an understandable condition which is being asserted as true.
I encountered these today, and put my boolean algebra to use.
It turns out that logical implication (a ⇒ b) is equivalent to (¬a ∨ b). The latter is just the form of expression we have in the asserts above. So these are really just if-then’s compacted down into expression form. In English, they say:
If id->locktype is NO_LOCK, then locktype has to be SHARED_LOCK.
if locktype is RESERVED_LOCK, then id->locktype has to be SHARED_LOCK.
Perhaps a better way to do this would be a macro for the purpose:
#define IFTHEN(x, y) (!(x) || (y))
...
assert( IFTHEN( id->locktype == NO_LOCK, locktype == SHARED_LOCK ))
assert( IFTHEN( locktype == RESERVED_LOCK, id->locktype == SHARED_LOCK ))
or, if you like really messing with people’s heads:
#define IF(x) (!(x))
#define THEN(y) || (y)
...
assert( IF( id->locktype == NO_LOCK ) THEN ( locktype == SHARED_LOCK ))
assert( IF( locktype == RESERVED_LOCK ) THEN (id->locktype == SHARED_LOCK ))
Of course, you could also do the reasonable thing:
if (id->locktype == NO_LOCK) {
assert(locktype == SHARED_LOCK);
}
if (locktype == RESERVED_LOCK) {
assert(id->locktype == SHARED_LOCK);
}
If the assert code is compiled away, the entire if statement will be also, so there’s nothing lost, and much understandability is gained.
Comments
What about if I convert...
assert(obj_is_valid(x) || x->blah == STATE_INIT);
if (!obj_is_valid(x)) assert(x->blah == STATE_INIT);
...so IMO, if you really don't like boolean algebra, you want something like assert_if();. Ie.
assert_if(!obj_is_valid(x), x->blah == STATE_INIT);
...both of which go away with NDEBUG set.
if ( !pointer || pointer->thing == BLAH_BLAH )
For example.
if ( horrible_condition )
{
Log ( "blah blah blah, %d", horrible_condition );
exit ( EXIT_FAILURE );
}
Sorry if that doesn't indent - I think that this comment board strips leading whitespace from lines.
GOD no, don't make #defines like that. #defines for virtually any purpose besides making simple literals are a pox. And even for simple literals, consider using stuff like const shorts and enums instead of #defines.
And I generally agree with you that #defines can be hugely abused. Sometimes they are the lesser of two evils.
Add a comment: