9 Code Smells of Preprocessor Use

October 4, 2011 — 31 Comments

With few exceptions, using the C preprocessor is a code smell. C++ programmers have had this beat into them: “Don’t use the preprocessor to do something the language itself provides.” Unfortunately, more than a few Objective-C programmers have yet to get that message.

[This post is part of the Code Smells in Objective-C series.]

Here’s a handy command to run from Terminal. It examines source files from the current directory down, showing preprocessor use that you should double-check.

find . \( \( -name "*.[chm]" -o -name "*.mm" \) -o -name "*.cpp" \) -print0 | xargs -0 egrep -n '^\w*\#' | egrep -v '(import|pragma|else|endif|HC_SHORTHAND|MOCKITO_SHORTHAND)'

This command builds in some exceptions. For example, #import directives are vital. #pragma mark can be useful. …But I want to question pretty much everything else! Why does it matter? Because every time you use the preprocessor, what you see isn’t what you compile. And for #define macros used as constants, there are certain pitfalls to avoid — when we could just avoid them altogether.

Here are some common preprocessor idioms, and how to replace them:

  1. #include
  2. Macros
  3. Constants: Numeric constants
  4. Constants: Ascending integer constants
  5. Constants: String constants
  6. Conditional compilation: Commenting out code
  7. Conditional compilation: Switching between experiments
  8. Conditional compilation: Switching between staging and production URLs
  9. Conditional compilation: Supporting multiple projects or platforms

1. #include

Let’s start with a simple one that comes from our C heritage:

Smell

#include "foo.h"

Unless you’re delivering platform-agnostic C or C++ code, there’s no reason to use #include, along with the accompanying include guards. Use #import instead; it eliminates the need for those #ifndef include guards.

2. Macros

Smell

#define WIDTH(view) view.frame.size.width

Just because you’re in Objective-C doesn’t mean you can’t use plain C functions! Unless your macro relies on preprocessor magic like __LINE__, rewrite it as a standalone function. (And even then, have your macro call another function and shift as much as you can to the function.)

And this isn’t your dad’s C! The C language continues to adopt small pieces of C++. One of these is the ability to inline functions:

static inline CGFloat width(UIView *view) { return view.frame.size.width; }

3. Constants: Numeric constants

Now we begin a set of preprocessor smells around constants. Using constants instead of repeating literal values is commendable. Using #define to create constants is not.

Smell

#define kTimeoutInterval 90.0

If a constant is used only within a single file, make it a static const. We give it an explicit type that adds to its semantic meaning. For that matter, the numeric literal can be expressed more simply if you like, because the explicit type clarifies the acceptable domain of values. So here’s what we get instead:

static const NSTimeInterval kTimeoutInterval = 90;

If a constant is shared across files, make it available it the way you do with everything else: create a declaration in the header file, and a definition in one implementation file. (Of course, you’re following Apple’s coding guidelines and using prefixes on your names. Right?) So the .h file has the declaration:

extern const NSTimeInterval JMRTimeoutInterval;

And the .m file has the definition:

const NSTimeInterval JMRTimeoutInterval = 90;

4. Constants: Ascending integer constants

Smell

#define firstNameRow 0
#define lastNameRow 1
#define address1Row 2
#define cityRow 3
// etc.

Ascending integer constants are handy for coding table views, to determine which information falls on which cell. …That’s what enumerated types are for.

enum {
    firstNameRow,
    lastNameRow,
    address1Row,
    cityRow,
    // etc.
};

Enumerated types make it easy to rearrange the order or add new values. In general, people use #defines because it’s easier to construct a dangerous macro than a safe constant. But here’s a case where what the language provides is not only safer, but easier.

An enumerated type doesn’t have to be named. But if you pass any of these values as arguments, you’ll want to define a type name to increase compiler checks and add semantic meaning. Rather than having to write “enum Address” everywhere you want to use the “Address” enumerated type, it’s common to create a typedef like this:

typedef enum {
    firstNameRow,
    lastNameRow,
    address1Row,
    cityRow,
    // etc.
} AddressRow;

5. Constants: String constants

Smell

#define JMRResponseSuccess @"Success"

As with numeric constants, use the language to define a constant. Only this time, we’re defining a constant string, which is really an object, which is expressed in Objective-C as a pointer. So we want to define a constant pointer.

Constant strings are often shared across multiple files, so here’s how to declare the constant in the .h file:

extern NSString *const JMRResponseSuccess;

The definition in the .m file is then:

NSString *const JMRResponseSuccess = @"Success";

6. Conditional compilation: Commenting out code

Conditional compilation, in its various flavors (#if, #ifdef, etc.), is a way to selectively enable or disable chunks of code. It’s used for different purposes, but it’s always a blunt instrument.

Smell

#if 0
...
#endif

In the old days of C, the only form of commenting was /**/. To comment out a chunk of code, you’d add /* in front and */ at the end. Then someone discovered that this didn’t work if the code already contained a comment. What to do? The answer at the time was to use the preprocessor: wrapping the code in #if 0 did the trick.

But that was a long time ago, before the dawn of modern IDEs and their color-coded ways. The color-coding helps us visually parse code more easily… but not for this. Even though there’s a 0 in this case, in general the IDE can’t know whether to show that conditional compilation has removed a chunk of code in a source file. So there’s no visual indicator that the code is commented out! It looks just like the rest of the code.

Fast-forward both C and Xcode to the present day. C has continued to evolve, and adopted the // commenting style from C++. Xcode takes advantage of this, and offers a “Comment Selection” command in the menus. Just press ⌘/ to comment out a section of code: Xcode will add // to the beginning of each selected line, color-coding them as comments. Pressing ⌘/ again reverses the process, bringing the code back.

So Xcode makes it easy to enable and disable code. But there’s another problem that we’ll get into in the following section: commenting-out code is fine if it’s temporary and you plan to clean it up soon. But too often, it’s just left there to rot…

7. Conditional compilation: Switching between experiments

Smell

#if EXPERIMENT
...
#else
...
#endif

Sometimes, you’re coding experimentally. Or you want a quick way to switch back-and-forth between two approaches. That’s fine.

But at some point, a decision is made. The experimental approach is validated, and you’re ready to ship. Clean up after yourself! Unless there’s some important historical reason to keep the rejected code as a comment, delete it. And if you choose to keep it, get rid of the preprocessor directives. Turn it into a real comment with explanation, not just code.

8. Conditional compilation: Switching between staging and production URLs

Smell

#if STAGING
static NSString *const fooURLString = @"https://dev.foo.com/services/fooservice";
#else
static NSString *const fooURLString = @"https://foo.com/services/fooservice";
#endif

When you develop service-based applications, you want to be able to specify whether you’re talking to the real production service, or to a staging service.

For simple apps with few URLs, I create a class for the URLs, and access them through methods:

- (NSString *)fooURLString
{
    DebugSettings *debugSettings = [self debugSettings];
    if ([debugSettings usingStaging])
        return @"https://dev.foo.com/services/fooservice";
    else
        return @"https://foo.com/services/fooservice";
}

For complicated apps that talk to many services, consider putting URLs into a plist instead. See Switching from Staging URL to Production URL for a plist example.

9. Conditional compilation: Supporting multiple projects or platforms

Smell

#if PROJECT_A
...
#else
...
#endif

When you have code that’s shared across multiple projects (or multiple platforms), it’s tempting to sneak in project-specific extensions into a shared source file. It may seem expedient, but it pollutes the source and conceals opportunities for unifying the code.

We work in an object-oriented language, so let’s use OO patterns, shall we? The basic strategy is to rework the methods containing project-specific code into Template Methods, with project-specific operations provided by project-specific subclasses.

Mechanics

  • Create a subclass for each project variant.
  • In each project, add the subclass for that project.
  • Compile each project.
  • Create a factory method that uses #if to create the right subclass. (We’re introducing one use of the preprocessor so that we can eliminate the others.)
  • Find each place that instantiates the original class. Have it call the factory method instead.
  • Compile and test each project.
  • For each conditionally compiled section:
    • Perform Extract Method to determine the required signature.
    • Move each platform-specific section of the body down to the platform-specific subclass, until the method at the base class is empty.
    • Compile and test each project.
  • Look for Duplicated Code within each subclass, and across the subclasses.

If you end up with multiple platform-specific subclass hierarchies across your code, you may find opportunities to use the Bridge pattern.

Avoid the C preprocessor!

Again, execute this command in Terminal to find the potentially offending preprocessor directives in your code. How many do you find? Can you reduce them? Are the ones that remain justified?

Remember: Don’t use the preprocessor to do something the language itself provides!

Question: Where do you still find the preprocessor helpful? What are the alternatives? Leave a comment below.

[This post is part of the Code Smells in Objective-C series.]

Disclosure of Material Connection: Some of the links in the post above are "affiliate links." This means if you click on the link and purchase the item, I will receive an affiliate commission. Regardless, I only recommend products or services I use personally and believe will add value to my readers. I am disclosing this in accordance with the Federal Trade Commission’s 16 CFR, Part 255: "Guides Concerning the Use of Endorsements and Testimonials in Advertising."

Enjoyed this article? Sign up to get future articles by email.

Email Address:

31 responses to 9 Code Smells of Preprocessor Use

  1. Hi John,
    Just wanted to say thank you for (8), switching between development and production urls. I’ve just recently hit a point in a small side project where this is needed. My initial thought was to do this the smelly way you mentioned which just seemed to junk up the code. I’ll look further into your suggestion of using a plist.
    Thanks!
    Cory Wheeler

    • oops, pardon the typo… your Jon… not John :)

    • Cory,
      This is one that snuck into my own code. In an effort to keep things simple, I just used a #if. Unfortunately, down the road, folks changed some server APIs without informing me, breaking my app. Users were unhappy, of course, saying, “Crashes, this app sucks!”
      All this could have been avoided if I had provided a hidden way to switch from production URLs to development URLs. The server folks could have used the released app to see that things weren’t right with their development changes.
      – Jon with no h :)

  2. Looking at this, I run into an annoying compiler issue. Perhaps this is in the C-spec, but it seems wrong:

    static const NSTimeInterval kSecondsInAMinute = 60;
    static const NSTimeInterval kDefaultTimeout = 5 * kSecondsInAMinute;
    

    The second line doesn’t compile because “error: initializer element is not constant”. However, given that kSecondsInAMinute is a static const, it seems like the compiler should understand that it is a constant. Bummer.

  3. I think I found an exception where using a macro is mandatory: to initialize compile time constants.

    #define COLOR_WITH_BYTES(r, g, b) (Color){r/255.0f, g/255.0f, b/255.0f}
    const Color color = COLOR_WITH_BYTES(14, 128, 255)

    Is there a way to avoid it?

    • David, I think you found a good exception: function calls (even inlined) are not permitted when doing static initialization of structs.

      The best way to avoid it would be to write out the initialization longhand, with all the /255.0f divisions.

      By the way, your macro as it stands is unsafe, which is another reason to be cautious with macros. Consider:

      const int redShift = 127;
      const Color color = COLOR_WITH_BYTES(redShift + 14, 128, 255);

      This won’t do what you expect, unless you wrap the macro arguments in parentheses, like (r)/255.0f

  4. I’m trying to take your advice and avoid preprocessors in favor of static consts. I have an NSString that I use as a string format for [NSString stringWithFormat:…]

    However, I’m getting the warning:

    Sending ‘const NSString *__strong’ to parameter of type ‘NSString *’ discards qualifiers

    How should I be handling this situation? I’d like to avoid the preprocessor, I don’t really want to add an instance variable so I can reuse this format.

    Suggestions?

    • Kenny, you declared your string const NSString * which is incorrect. It should be NSString *const (which prevents the pointer from being inadvertently changed to a different string).

      Came here searching for a solution to that warning message? I invite you to browse around — my About page is a good place to start.

  5. I definitely agree that #define are usually bad smells.

    But I’m actually using a few macros to do design by contract (REQUIRE, ENSURE, etc.) in my iOS code that I simply cut out in the release build. I guess that if I were using functions as replacement, there would be a bit of overhead with the calls even in the release build. Is this an acceptable use of #define?

    Would an inline empty function be the solution? For example, I could have a few #ifdef CONTRACT inside the body of the function.

    I’m well aware of NS(Parameter)Assert, etc. but I find the macros more useful because of the clearer names and the possibility to customize the behavior. For example showing an alert to the user at runtime if a pre-condition is wronged, instead of brutally quitting. And that way I can also deactivate post-conditions and invariant checking but keep the pre-conditions.

    Thanks!

    • Mickaël,

      Macros like that seem fine to me. It gives you a way to have temporary scaffolding during construction, which disappears for shipping.

      Actually, I’m curious to learn more about them, they sound interesting! Where did you come up with REQUIRE, ENSURE, etc.?

      • Mickaël might be referring to “Design by Contract” http://en.wikipedia.org/wiki/Design_by_contract

        I had one project where I used that from beginning to end and its probably the best tested least buggy code I have had the ongoing pleasure of working with. There is always a tradeoff it is a lot of up front scaffolding and certain frameworks like C# have built in support for it that made it easier. I found that the approach works well with agile and waterfall approaches, as with any process it works best with total team buy-in cause all it takes is one developer to tank the whole effort.

  6. Great series, Jon; thanks for sharing.

    I would also note that for item 7, the problem is probably best solved at the source control level via branches (especially when using Git).

  7. Hi Jon,

    Thanks for the great info for Preprocessor use. Is there any tool out there for counting the preprocessor usage?.

    Thanks,
    Raj

  8. Arkadiusz Młynarczyk December 4, 2013 at 6:32 am

    I would say that:

    typedef enum {
    firstNameRow,
    lastNameRow,
    address1Row,
    cityRow,
    // etc.
    } AddressRow;

    is a smell.

    Instead everybody should use

    typedef NS_ENUM(NSUInteger, AddressRow) {
    firstNameRow,
    lastNameRow,
    address1Row,
    cityRow,
    // etc.
    };

    That build in macro can save you a lot of trubble when you will switch to Objective-C++

  9. The preprocessor macro is a function that Objective-C DO provide. So, we don’t need to hesitate to use them. Whether we use them or not depends on what is important. Your smells count to some extent, but we have more important thing usually.

  10. I agree with Arkadiusz Młynarczyk about “typedef NS_ENUM(NSUInteger, AddressRow)” but would add that if it is for a bit mask you should use “NS_OPTIONS(type, name)” instead.

    Yepher

  11. Thanks Jon for this awesome post.

    one question: the definition of NSString constants is bad, but if I use the “extern NSString * const” way I need a header and a implementation file. In your youtube screencast of TDD you recommend to not use a header and implementation file in test cases.

    So how should I define string constants in my test cases?

    • You only need extern NSString * const if you’re defining a constant in one compilation unit but using it in another. When a string constant is used in only one compilation unit, define it as a static.

      But things are simpler still for test cases. If the string is obviously simple (like @"FOO") I just repeat it. If the string is more complicated, or it just makes the test read better, put it in a local variable.

  12. Hey Jon…thanks for the series!

    I noticed that you talk about defining string and numeric constants, but not other kinds of constants. I have an example where I defined a UIColor constant:

    #define EDITING_CELL_BACKGROUND_COLOR [UIColor colorWithRed:229.0/255.0 green:243.0/255.0 blue:250.0/255.0 alpha:1.0]

    Is this a case where using a macro doesn’t smell? If it does smell, how would you define the constant?

  13. What about static dictionaries / arrays?

    • Simon, the preprocessor is one way to approach simple code generation, and I sometimes use it for static collections. If the code generation gets much more complex, I’d favor scripts. (And part of the generated code would be a clear comment about DON’T EDIT THIS, INSTEAD CHANGE THE SCRIPT AND REGENERATE.)

  14. Gabriel Aubut-Lussier August 14, 2014 at 10:08 am

    Concerning your point 3, prefixes shouldn’t be only 2 characters long (this is the convention for Apple’s prefixes), they should be precisely 3 characters long.
    Here’s Apple’s relevant documentation : https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/Conventions/Conventions.html

Leave a Reply

*

Text formatting is available via select HTML.

<a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong> 

Have you Subscribed yet? Don't miss a post!