Let's take a silly example:

#define MAX_NUM_BITES 20
...
void live_another_day() {
  ...
  int num_bites = 0;
  ...
  eat_little_pig(&num_bites);
  if (num_bites>MAX_NUM_BITES) {
    printf("You shouldn't more than %d bites. Otherwise, you will get fat!");
    return;
  }
  ...
  eat_grandma(&num_bites);
  if (num_bites>MAX_NUM_BITES) {
    printf("You shouldn't more than %d bites. Otherwise, you will get fat!");
    return;
  }
  ...
  eat_red_riding_hood(&num_bites);
  if (num_bites>MAX_NUM_BITES) {
    printf("You shouldn't more than %d bites. Otherwise, you will get fat!");
    return;
  }
  ...
}

That example will read better if we define this macro:

#define EXIT_WITH_ERROR_MESSAGE_IF_FULL(num_bites)                            \
  if (num_bites>MAX_NUM_BITES) {                                              \
    printf("You shouldn't more than %d bites. Otherwise, you will get fat!"); \
    return;                                                                   \
  }

void live_another_day() {
  ...
  int num_bites = 0;
  ...
  eat_little_pig(&num_bites);
  EXIT_WITH_ERROR_MESSAGE_IF_FULL(num_bites)
  ...
  eat_grandma(&num_bites);
  EXIT_WITH_ERROR_MESSAGE_IF_FULL(num_bites)
  ...
  eat_red_riding_hood(&num_bites);
  EXIT_WITH_ERROR_MESSAGE_IF_FULL(num_bites)
  ...
}

My question is: should I take it a step further and not even pass num_bites to the macro?

#define EXIT_WITH_ERROR_MESSAGE_IF_FULL()                                     \
  if (num_bites>MAX_NUM_BITES) {                                              \
    printf("You shouldn't more than %d bites. Otherwise, you will get fat!"); \
    return;                                                                   \
  }

Or is that not good coding practice?

Subsidiary question: is it good coding practice not to follow the macro call with a ';' ?

5 Replies 5

Let’s deal with your second question first, since it is easy and readily addressed. It is common to design a macro so that using it “looks like” a statement when a ; is appended. So a macro foo could be invoked as foo(0, 1, 2);. Now, if the macro has its own semicolon or is a braced group, as in { thing1; thing2; }, then having an extra semicolon can cause problems when the macro is used with if statements or other constructs. The common idiom for handling this is do wrap the macro in a do statement:

#define foo(arg0, arg1, arg2) do { \
        statement0;                \
        statement1;                \
    } while (0)

Then the macro needs a semicolon after it to be a complete statement, and it will work nicely as a substatement to other statements.

While the definition might look a little awkward at first (Why does this superfluous do with an always-false while (0) appear?), experienced programmers will recognize this common idiom.

As to your first question, will the macro ever be used with a different value? If so, then it needs an argument to say what value is to be used. Here, “ever” refers not just to the code you are writing at the moment but to whatever the macro might be used for in the future.

Sometimes this is not known and is hard to speculate about. Often, it is good to make the values any piece of source code works with visible and well-defined. So making a value an argument to the macro, instead of something drawn from some unrelated place in the environment, is good practice.

There is not a hard and fast rule about this. Sometimes a program has a significant environment to it with file scope identifiers that are solid and well-known to programmers working on it. For example, stdin is a well-known name, and people generally have found it useful to hard-code into some macros, while other macros take the stream to use as an argument.

So, whether to make something an argument to a macro or to take it from elsewhere is both situation-dependent and a matter of judgement and opinion.

Creating function-like macros to replace chunks of code or program flow control is almost always bad practice.

Good practice in this case would be to rewrite the code so that it is generic and can be executed in a loop, instead of repeating the same code over and over with just slightly different input. For example you can regard the number of "eat" functions as tasks or a state machine etc.

To achieve that, we can make a local array of function pointers:

typedef void eat_func_t (int* bites); // expected function template of these functions

static eat_func_t* const eat[] = // read-only array of function pointers to named functions
{
  &eat_little_pig,
  &eat_grandma,
  &eat_red_riding_hood,
};

const size_t n = sizeof(eat)/sizeof(*eat); // size of the array

Then just call that array from within a loop:

#define MAX_NUM_BITES 20

void live_another_day() 
{
  typedef void eat_func_t (int* bites);

  static eat_func_t* const eat[] =
  {
    eat_little_pig,
    eat_grandma,
    eat_red_riding_hood,
  };
  const size_t n = sizeof(eat)/sizeof(*eat);

  for(size_t i=0; i<n; i++)
  {
    int num_bites = 0;
    ...
    eat[i](&num_bites);
    
    if (num_bites>MAX_NUM_BITES) {
        printf("You shouldn't eat more than %d bites. Otherwise, you will get fat!", 
               MAX_NUM_BITES);
        return;
    }
  }
}

Though in general it is bad to mix in error presentation inside algorithms - it is better to just return a result code and let the caller print the error if required.

@EricPostpischil "experienced programmers will recognize this common idiom." I remember old MSVC compilers used to issue a warning like "condition is always false", which was annoying!

@Lundin Love the way you refactored the code but wonder if that's always practical or even advisable. Can you say more about why you believe "creating function-like macros to replace chunks of code or program flow control is almost always bad practice"?

Your Reply

By clicking “Post Your Reply”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.