Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It only takes a minute to sign up.
Sign up to join this community
This doesn't seem quite right, but it's the best I could do. Is there a way to do this better? My goal is to use DEBUG when I'm developing by doing a 'cc -DDEBUG' at the prompt.
#ifndef DEBUG
#define ptrValues ptrvaluesRemote
#else
#define ptrValues ptrValuesLocal
#endif
ptrLinks = (struct sLinkDescriptionLayout*) ptrValues();
Perhaps use NDEBUG (as used in assert.h) instead of making up your own. Also note that unless you put an ifdef around the two functions you might get a linkage warning (as one of the functions might be unused). A warning is undesirable, but so is putting the ifdef around the functions, which will result in one of the functions not being compiled and in time the function may rot.
Note that you might achieve the same end using the tool-chain to modify the symbol table.
Another alternative would be to name the two functions the same in two separate files and link one or the other as required - that would be a cleaner solution, but would need documenting.
Also...
Perhaps make ptrLinks and ptrValuesEtc the same type so that a cast is unnecessary.
Consider whether ptrLinks and ptrValuesEtc are correctly named:
#ifdef but I am concerned about having the function code branch, one for production and one for debug. It's a tangential topic, but a good one.
\$\endgroup\$
– EhevuTov
Aug 3 '12 at 20:01
-DNDEBUG) separate from the rest of your debugging (so I agree 100% with @LokiAstari — and upvoted his comment too).
\$\endgroup\$
– Jonathan Leffler
Aug 18 '12 at 5:30
Not much prettier, but maybe preferable depending on how you need to use it later:
#ifdef DEBUG
#define DEBUG_FLAG 1
#else
#define DEBUG_FLAG 0
#endif
ptrLinks = (struct sLinkDescriptionLayout*) DEBUG_FLAG?ptrValuesLocal():ptrValuesRemote();
The C preprocessor is simultaneously fantastic and terrible.
I recommend making the function name macro into a function-like macro instead of an object-like macro — give it an empty argument list. Also, it is usually best to keep the logic positive, so use #ifdef rather than #ifndef. This is hardly critical here, but in more complex situations, it is a good guideline to follow.
These observations lead to:
#if defined(DEBUG)
#define ptrValues() ptrValuesLocal()
#else
#define ptrValues() ptrvaluesRemote()
#endif
ptrLinks = (struct sLinkDescriptionLayout*) ptrValues();
Personally, I would also have:
typedef struct sLinkDescriptionLayout sLinkDescriptionLayout;
Then I wouldn't need the struct in the cast:
ptrLinks = (sLinkDescriptionLayout *)ptrValues();
I'd probably go for a shorter name, too, (perhaps LinkDesc, but it depends on what the other names in the code are) but that's very much less critical.
