views:

3756

answers:

15

This may be a matter of style, but there's a bit of a divide in our dev team and I wondered if anyone else had any ideas on the matter...

Basically, we have some debug print statements which we turn off during normal development. Personally I prefer to do the following:

\\---- SomeSourceFile.cpp ----

#define DEBUG_ENABLED (0)

...

SomeFunction()
{
    int someVariable = 5;

#if(DEBUG_ENABLED)
    printf("Debugging: someVariable == %d", someVariable);
#endif
}

Some of the team prefer the following though:

\\---- SomeSourceFile.cpp ----

#undef DEBUG_ENABLED
\\#define DEBUG_ENABLED

...

SomeFunction()
{
    int someVariable = 5;

#ifdef DEBUG_ENABLED
    printf("Debugging: someVariable == %d", someVariable);
#endif
}

...which of those methods sounds better to you and why? My feeling is that the first is safer because there is always something defined and there's no danger it could destroy other defines elsewhere.

A: 

#if gives you the option of setting it to 0 to turn off the functionality, while still detecting that the switch is there.
Personally I always #define DEBUG=1 so I can catch it with either an #if or #ifdef

Martin Beckett
This fails, since #define DEBUG=0 will now not run #if but will run #ifdef
tloach
Thats the point, I can either remove DEBUG completely or just set it to 0 to disable it.
Martin Beckett
+3  A: 

I myself prefer:

#if defined(DEBUG_ENABLED)

Since it makes it easier to create code that looks for the opposite condition much easier to spot:

#if !defined(DEBUG_ENABLED)

vs.

#ifndef(DEBUG_ENABLED)
Jim Buck
Personally I think it's easier to miss that little exclamation mark!
Jon Cage
With syntax highlighting? :) In syntax highlighting, the "n" in "ifndef" is much harder to spot since it's all the same color.
Jim Buck
Okay I meant #ifndef is easier to spot than #if !defined when you're comparing against #if defined .. but given all #if defined/#if !defined vs #ifdef/#ifndef, either is equally miss-readable!
Jon Cage
+1  A: 

The first seems clearer to me. It seems more natural make it a flag as compared to defined/not defined.

directrixx
A: 

Alternatively, you can declare a global constant, and use the C++ if, instead of the preprocessor #if. The compiler should optimize the unused branches away for you, and your code will be cleaner.

Here is what C++ Gotchas by Stephen C. Dewhurst says about using #if's.

Dima
That's a lousy solution, it has the following problems:1. Only works in functions, you can't remove unneeded class variables, etc2. Compilers may throw warnings about unreachable code3. Code in the if still needs to compile, which means you have to keep all your debug functions defined, etc.
Don Neufeld
First the question was specifically about debug printfs, so unneeded class variables are not an issue here. Second, given the capabilities of modern compilers you should use #ifdefs as little as possible. In most cases you can use build configurations or template specializations instead.
Dima
A: 

I've always used #ifdef and compiler flags to define it...

tloach
Any particular reason (out of curiosity)?
Jon Cage
To be honest I never thought about it - just how it's been done places I've worked. It does give the advantage that instead of making a code change for a production build all you have to do is 'make DEBUG' for debug, or 'make PRODUCTION' for regular
tloach
+9  A: 

I think it's entirely a question of style. Neither really has an obvious advantage over the other.

Consistency is more important than either particular choice, so I'd recommend that you get together with your team and pick one style, and stick to it.

Derek Park
+2  A: 

Both are exactly equivalent. In idiomatic use, #ifdef is used just to check for definedness (and what I'd use in your example), whereas #if is used in more complex expressions, such as #if defined(A) && !defined(B).

zvrba
The OP wasn't asking for which is better between "#ifdef" and "#if defined" but rather between "#ifdef/#if defined" and "#if".
shank
+1  A: 

#if and #define MY_MACRO (0)

Using #if means that you created a "define" macro, i.e., something that will be searched in the code to be replaced by "(0)". This is the "macro hell" I hate to see in C++, because it pollutes the code with potential code modifications.

For example:

#define MY_MACRO (0)

int doSomething(int p_iValue)
{
   return p_iValue + 1 ;
}

int main(int argc, char **argv)
{
   int MY_MACRO = 25 ;
   doSomething(MY_MACRO) ;

   return 0;
}

gives the following error on g++:

main.cpp|408|error: lvalue required as left operand of assignment|
||=== Build finished: 1 errors, 0 warnings ===|

Only one error.

Which means that your macro successfully interacted with your C++ code: The call to the function was successful. In this simple case, it is amusing. But my own experience with macros playing silently with my code is not full of joy and fullfilment, so...

#ifdef and #define MY_MACRO

Using #ifdef means you "define" something. Not that you give it a value. It is still polluting, but at least, it will be "replaced by nothing", and not seen by C++ code as lagitimate code statement. The same code above, with a simple define, it:

#define MY_MACRO

int doSomething(int p_iValue)
{
   return p_iValue + 1 ;
}

int main(int argc, char **argv)
{
   int MY_MACRO = 25 ;
   doSomething(MY_MACRO) ;

   return 0;
}

Gives the following warnings:

main.cpp||In function ‘int main(int, char**)’:|
main.cpp|406|error: expected unqualified-id before ‘=’ token|
main.cpp|399|error: too few arguments to function ‘int doSomething(int)’|
main.cpp|407|error: at this point in file|
||=== Build finished: 3 errors, 0 warnings ===|

So...

Conclusion

I'd rather live without macros in my code, but for multiple reasons (defining header guards, or debug macros), I can't.

But at least, I like to make them the least interactive possible with my legitimate C++ code. Which means using #define without value, using #ifdef and #ifndef (or even #if defined as suggested by Jim Buck), and most of all, giving them names so long and so alien no one in his/her right mind will use it "by chance", and that in no way it will affect legitimate C++ code.

Post Scriptum

Now, as I'm re-reading my post, I wonder if I should not try to find some value that won't ever ever be correct C++ to add to my define. Something like

#define MY_MACRO @@@@@@@@@@@@@@@@@@

that could be used with #ifdef and #ifndef, but not let code compile if used inside a function... I tried this successfully on g++, and it gave the error:

main.cpp|410|error: stray ‘@’ in program|

Interesting. :-)

paercebal
I agree that macros can be dangerous, but that first example would be fairly obvious to debug and of course it only gives one error. Why would you expect more? I've seen much nastier errors as a result of macros...
Jon Cage
It's true the difference between one solution and another is almost trivial. But in this case, as we are talking about two competing coding styles, then even the trivial can't be ignored, because after that, all that is left is personal taste (and at that point, I believe it shouldn't be normalized)
paercebal
A: 

A little OT, but turning on/off logging with the preprocessor is definitely sub-optimal in C++. There are nice logging tools like Apache's log4cxx which are open-source and don't restrict how you distribute your application. They also allow you to change logging levels without recompilation, have very low overhead if you turn logging off, and give you the chance to turn logging off completely in production.

David Nehme
I agree, and we actually do that in our code, I just wanted an example of something you might use #if etc. for
Jon Cage
+11  A: 

My initial reaction was '#ifdef, of course', but I think #if actually has some significant advantages for this - here's why:

First, you can use 'DEBUG_ENABLED' in preprocessor and compiled tests. Example - Often, I want longer timeouts when debug is enabled, so using #if, I can write this

  DoSomethingSlowWithTimeout(DEBUG_ENABLED? 5000 : 1000);

... instead of ...

#ifdef DEBUG_MODE
  DoSomethingSlowWithTimeout(5000);
#else
  DoSomethingSlowWithTimeout(1000);
#endif

Second, you're in a better position if you want to migrate from a #define to a global constant. #defines are usually frowned on by most C++ programmers.

And, Third, you say you've a divide in your team. My guess is this means different members have already adopted different approaches, and you need to standardise. Ruling that #if is the preferred choice means that code using #ifdef will compile -and run- even when DEBUG_ENABLED is false. And it's much easier to track down and remove debug output that is produced when it shouldn't be than vice-versa.

Oh, and a minor readability point. You should be able to use true/false rather than 0/1 in your #define, and because the value is a single lexical token, it's the one time you don't need parentheses around it.

#define DEBUG_ENABLED true

instead of

#define DEBUG_ENABLED (1)
Roddy
The constant might not be used to enable/disable debugging, so triggering a #ifdef with a #define to 0 could be not so benign. As for true/false, those were added in C99 and don't exist in C89/C90.
Michael Carman
Micheal: He/she was advocating _against_ the use of #ifdef ?!
Jon Cage
...good point about the true / false - especially as our embedded platform doesn't actually define bool!
Jon Cage
@Jon Cage - what platform? Some of them do, but in strange places.
detly
+2  A: 

It's a matter of style. But I recommend a more concise way of doing this:

#ifdef USE_DEBUG
#define debug_print printf
#else
#define debug_print
#endif

debug_print("i=%d\n", i);

You do this once, then always use debug_print() to either print or do nothing. (Yes, this will compile in both cases.) This way, your code won't be garbled with preprocessor directives.

If you get the warning "expression has no effect" and want to get rid of it, here's an alternative:

void dummy(const char*, ...)
{}

#ifdef USE_DEBUG
#define debug_print printf
#else
#define debug_print dummy
#endif

debug_print("i=%d\n", i);
Lev
Perhaps the printing macro wasn't the best example afterall - we actually do this already in our codebase for our more standard debug code. We use the #if / #ifdefined bits for areas which you might want to turn on extra debugging..
Jon Cage
+3  A: 

For the purposes of performing conditional compilation, #if and #ifdef are almost the same, but not quite. If your conditional compilation depends on two symbols then #ifdef will not work as well. For example, suppose you have two conditional compilation symbols, PRO_VERSION and TRIAL_VERSION, you might have something like this:

#if defined(PRO_VERSION) && !defined(TRIAL_VERSION)
...
#else
...
#endif

Using #ifdef the above becomes much more complicated, especially getting the #else part to work.

I work on code that uses conditional compilation extensively and we have a mixture of #if & #ifdef. We tend to use #ifdef/#ifndef for the simple case and #if whenever two or more symbols are being evaluation.

Mike Thompson
+3  A: 

#ifdef just checks if a token is defined, given

#define FOO 0
then
#ifdef FOO // is true
#if FOO // is false, because it evaluates to "#if 0"

Terence Simpson
That is true, but which is better?
Jon Cage
+1  A: 

We have had this same problem across multiple files and there is always the problem with people forgetting to include a "features flag" file (With a codebase of > 41,000 files it is easy to do).

If you had feature.h:

#ifndef FEATURE_H
#define FEATURE_H

// turn on cool new feature
#define COOL_FEATURE 1

#endif // FEATURE_H

But then You forgot to include the header file in file.cpp:

#if COOL_FEATURE
    // definitely awesome stuff here...
#endif

Then you have a problem, the compiler interprets COOL_FEATURE being undefined as a "false" in this case and fails to include the code. Yes gcc does support a flag that causes a error for undefined macros... but most 3rd party code either defines or does not define features so this would not be that portable.

We have adopted a portable way of correcting for this case as well as testing for a feature's state: function macros.

if you changed the above feature.h to:

#ifndef FEATURE_H
#define FEATURE_H

// turn on cool new feature
#define COOL_FEATURE() 1

#endif // FEATURE_H

But then you again forgot to include the header file in file.cpp:

#if COOL_FEATURE()
    // definitely awseome stuff here...
#endif

The preprocessor would have errored out because of the use of an undefined function macro.

Brent Priddy
A: 

They're both hideous. Instead, do this:

#ifdef DEBUG
#define D(x) do { x } while(0)
#else
#define D(x)
#endif

Then whenever you need debug code, put it inside D();. And your program isn't polluted with hideous mazes of #ifdef.

R..