Good Preprocessor Usage

I was digging through some code and came across the following preprocessor construct:

#if FOO == 0
void foo(int bar)
{
#endif
#if FOO != 0
void foo(float baz)
{
#endif

As a general rule, conditionally-compiled code makes the hairs on the back of my neck stand up, but given that this is code for embedded systems, and the same basic code runs on a number of different hardware platforms, I understand that it is sometimes a necessary evil.  The problem is, people don't always treat preprocessing directives as what they really are: code.  They're instructions processed by a computer, and deserve the same good coding practices that one would apply to the rest of their source code.  Preprocessor code may actually deserve more attention to best practices due to the difficult nature of debugging it.  Taking the above as a case study, here are a few thoughts on how to improve this:

Magic Numbers
0 is a magic number, and as everybody knows, magic numbers are a no-no in code.  Does the quantity of FOO have any significance?  If so, then a name that suggests a count, index, etc would be apppriate, like FOO_COUNT.  Is FOO an enumerated type?  Then use named constants, like FOO_A, FOO_B for the values of FOO.  Is it a boolean flag?  If so, use the classic TRUE and FALSE for definitions, and make the #if read like a boolean: #if FOO or #if FOO == TRUE.

Disjoint, yet mutually exclusive cases
These cases, while mutually exclusive, are not guaranteed to be so by the branching construct itself.  They are guaranteed so by the conditional statement, which can be (and is far more likely to be) changed to not be mutually exclusive, while appearing to retain this exclusivity.  Code that appears to be something it's not is a prime candidate for bugs.  This is far more likely to be the case  if FOO is a non-binary type (quantitative or enumerated).  Imagine you realize you want to add a case for #if FOO == FOO_A.  Then you end up with:

#if FOO == 0
void foo(int bar)
{
#endif
#if FOO != 0
void foo(float baz)
{
#endif
#if FOO == 2
void foo(char *bap)
{
#endif

Now, if FOO is set to 2, you have the following nasty syntax error in your code after preprocessing:

void foo(float baz)
{
void foo(char *bap)
{

So maybe the developer was smart enough to remove the FOO != 0 case, so we don't have this double declarator problem.  But what if FOO is defined as 1?  Now we have a funciton with no declarator.  The real solution is to use the #elseif to force mutual exclusion of FOO clauses, and a #else at the end to give a default case or throw a compiler error for invalid FOO values:

#if FOO == FOO_A
do some A stuff
#elif FOO == FOO_B
do some B stuff
#else
#  error "Invalid value for FOO."
#endif

Or, for a simple boolean/binary system, we can use the same sort of logic that C uses.  A 0 value is false, everything else is true.

#if BAR
do some BAR stuff
#else
do some non-BAR stuff
#endif

But I recommend defining BAR as TRUE or FALSE instead of 1 or 0, to be clear.  This also allows you to take better advantage of IDE features.  For example, MS Visual Studio shows you the definition when you hover over a symbol name.  Seeing "#define BAR TRUE" gives you a much better idea of how to use BAR than does "#define BAR 1".

So what this really all boils down to is this: preprocessor code is just like any code.  Treat it with the respect it deserves, and apply the same good practices to it as you do to the rest of the code you write.

Comments

Popular posts from this blog

Easier sed than done

Welcome!