Code Smells

Here is a little snippet of C code which recently reared up its ugly head and bit me, these sort of problems are often termed Code Smells. No, I’m not going to say who wrote it…

#ifdef LCFG_OS_rh9
    ...
#endif
#if defined(LCFG_OS_fc3) || defined(LCFG_OS_fc5) || defined(LCFG_OS_fc6) || defined(LCFG_OS_sl5)
    ...
#endif

There are a couple of problems here, the biggest and most obvious of which is the maintenance issue. Every time a new platform is introduced this code will need to be modified to add a check for another LCFG_OS_ macro. It’s a bug lurking in the dark waiting to pounce on the unwary, in particular it makes it hard for any external users to take the code and use it elsewhere.

The deeper problem, and the reason I consider this a “code smell”, is that the code checks for the wrong thing. The real requirement is to check which API is being used by a library. Although, in the normal case, the choice conveniently maps onto the platform there is no reason why the older version of the software couldn’t be used on a more recent operating system. What is really needed here is a check for the required API. The code now reads:

#ifdef OLD_API
   ...
#else
   ...
#endif

This automatically chooses the up-to-date API unless the user has specified that the older API is required. This could be done manually or through automatic detection incorporated into the build scripts. Ideally the headers for the library would have a macro defined which specified the API being used but in this case there does not seem to be anything like that available.

Comments are closed.