8

I'm trying to implement logging which produce no overhead when not needed (i.e. no method call should be performed at all). I want NO overhead because it's low-latency code. I just added #define ENABLE_LOGS to my header class and now it looks like that (you can ignore details)

#pragma once

#include <string>
#include <fstream>

#define ENABLE_LOGS

namespace fastNative {

    class Logger
    {
    public:
        Logger(std::string name_, std::string fileName, bool append = false);
        ~Logger(void);
        void Error(std::string message, ...);
        void Debug(std::string message, ...);
        void DebugConsole(std::string message, ...);
        void Flush();
        static Logger errorsLogger;
        static Logger infoLogger;
    private:
        FILE* logFile;
        bool debugEnabled;
    };

}

Every time I need to use some method I should surround it like that:

#ifdef ENABLE_LOGS
    logger.Debug("seq=%6d len=%4d", seq, length_);
#endif

It's error-phrone (i can forgot to surround) and makes code dirty. Can I fix my code somehow not to use #ifdef every time?

In C# I like Conditional I guess I need something like that for c++.

Oleg Vazhnev
  • 23,239
  • 54
  • 171
  • 305
  • 1
    Find any assert.h - even the one at http://stackoverflow.com/questions/9701229/c-assert-implementation-in-assert-h for an example of how to make code that you can turn off with one flag in one place. – Scott Mermelstein Jul 15 '13 at 21:11
  • 2
    You can conditionally `#define` the Debug method to have an (`inline`) empty implementation in the header, then rely on the optimizer to remove function calls that do nothing (this assumes any arguments don't have side effects). The more traditional way is to use preprocessor macros directly, which is more direct but can be slightly more error-prone. – Cameron Jul 15 '13 at 21:12
  • I'm assuming you don't want the arguments to be evaluated either. This complicates things significantly if you want a clean C++ look. Time for some macros! – Cory Nelson Jul 15 '13 at 21:20
  • use polymorphism and have an implementation that does nothing and select the correct implementation using your macro. – andre Jul 15 '13 at 21:22
  • 1
    It is somehow interesting that you are concerned with performance of the logger but your logging functions take `std::string` by value. At any rate, take a look at any of the existing logging libraries, you are probably not going to do any better than that – David Rodríguez - dribeas Jul 15 '13 at 21:43
  • it would also be a good idea to look at pre-existing frameworks (search, e.g. see here, http://stackoverflow.com/questions/1736295/c-logging-framework-suggestions) before doing anything yourself, as you might get ideas from what's out there. – TooTone Jul 16 '13 at 08:58
  • You can't change the logging verbosity at run-time with such a design. Which means that if your application misbehaves at run-time you won't be able to tell it to log more. You'd have to deploy another version built with logging from exactly the same source files (which is easy enough if you tag/label them in the source control). Which may be problematic in terms of maintenance. – Maxim Egorushkin Jul 17 '13 at 19:50

5 Answers5

13

First of all it would make sense to have a look to see what's out there already. This is a common problem and many people will have solved it before. E.g., see stackoverflow question C++ logging framework suggestions, and Dr Dobbs A Highly Configurable Logging Framework In C++.

If you do roll your own, you should get some good ideas from having done this. There are several approaches I've used in the past. One is to make the statement itself conditionally defined

#ifdef ENABLE_LOGS
#define LOG(a,b,c) logger.Debug(a, b, c)
#else
#define LOG(a,b,c)
#endif

Another approach is to conditionally define the logging class itself. The non-logging version has everything as empty statements, and you rely on the compiler optimizing everything out.

#ifdef ENABLE_LOGS

class Logger
{
public:
    Logger(std::string name_, std::string fileName, bool append = false);
    ~Logger(void);
    void Error(std::string message, ...);
    void Debug(std::string message, ...);
    void DebugConsole(std::string message, ...);
    void Flush();
    static Logger errorsLogger;
    static Logger infoLogger;
private:
    FILE* logFile;
    bool debugEnabled;
};

#else

class Logger
{
public:
    Logger(std::string name_, std::string fileName, bool append = false) {}
    ~Logger(void) {}
    void Error(std::string message, ...) {}
    void Debug(std::string message, ...) {}
    void DebugConsole(std::string message, ...) {}
    void Flush() {}
};

#endif

You could put your Logger implementation for ENABLE_LOGS in a cpp file under control of the macro. One issue with this approach is that you would want to be sure to define the interface so the compiler could optimize everything out. So, e.g., use a C-string parameter type (const char*). In any case const std::string& is preferable to std::string (the latter ensures there's a string copy every time there's a call).

Finally if you go for the first approach, you should encapsulate everything in do() { ... } while(0) in order to ensure that you don't get bizarre behavior when you use your macro where a compound statement might be expected.

Community
  • 1
  • 1
TooTone
  • 7,129
  • 5
  • 34
  • 60
  • i not always call variable `logger`. I use different names and sometimes I have more than one logger visible – Oleg Vazhnev Jul 16 '13 at 08:31
  • @javapowered in that case for the first approach you could have another parameter for the macro to specify which logger object. For the second approach you don't need to do anything different -- you just use the class as normal but if logging is turned off all the implementations are inline null and you should get no code in a release build. – TooTone Jul 16 '13 at 08:55
  • The second approach won't necessarily optimize everything out. For instance if you have something like `debug.Log(..., foo())`, where `Log` does nothing, then the function `foo` still has to be called (unless it's in the same translation unit and has no side effects). – Kaz Jul 17 '13 at 19:23
  • @Kaz agreed it's not perfect and I put one or two caveats in my answer. Yours is a good point which I'll add If I get time. – TooTone Jul 17 '13 at 20:02
7

There is one way (the way llvm does) to do this using macros.

#ifdef ENABLE_LOGS
#define DEBUG(ARG) do { ARG; } while(0)
#else
#define DEBUG(ARG)
#endif

Then use it as:

DEBUG(logger.Debug("seq=%6d len=%4d", seq, length_););
A. K.
  • 34,395
  • 15
  • 52
  • 89
  • What's the purpose of the do...while in the macro? – Code-Apprentice Jul 15 '13 at 21:17
  • 3
    + @Monad: That's in case you want to say `if(test) DEBUG(...); else ...`. It makes it a single statement. If it expanded to `{ARG;}` you would get a syntax error at the `else`. – Mike Dunlavey Jul 15 '13 at 21:20
  • And that's why this approach is terrible in general. – Bartek Banachewicz Jul 17 '13 at 16:33
  • Non sequitur. This is the only approach which is guaranteed to eliminate all evaluation from the arguments of the logging call when the logging is turned off at compile time, even if the arguments call external functions. – Kaz Jul 17 '13 at 19:24
  • 1
    @ Bartek Banachewicz : If this approach was terrible then compiler writers and standard library writer wouldn't be using it. The approach you have presented is a well known example of bad usage of #ifdefs. – A. K. Jul 17 '13 at 19:56
  • @AdityaKumar Your logical fallacy is [appeal to authority](https://yourlogicalfallacyis.com/appeal-to-authority). And you didn't even understand my approach properly, so please, stop introducing unnecessary noise. – Bartek Banachewicz Jul 17 '13 at 20:10
3

What I often see, is to use the #define to actually define the log calls, eg:

#define LOG_DEBUG(msg) logger.Debug(msg);

But you want to wrap the defines in a block that enables or disables your logging:

#ifdef ENABLE_LOGS
#define LOG_DEBUG(msg) logger.Debug(msg);
#else
#define LOG_DEBUG(msg)
#endif

You can call LOG_DEBUG anywhere in your code. If the logging is disabled, calling LOG_DEBUG ends up as a blank line in your final code.

Lochemage
  • 3,974
  • 11
  • 11
  • Your answer will be better if you make it a [variadic macro](http://stackoverflow.com/a/679993/315052). – jxh Jul 15 '13 at 21:17
  • There are several ways to go about defining them, it's a matter of preference for the most part. But, that is one nice solution. – Lochemage Jul 15 '13 at 21:42
0

A nice old trick is:

#ifdef ENABLE_LOGS
#define LOG Logger.Debug
#else
#define LOG (void)sizeof
#endif

Then the code:

LOG("seq=%6d len=%4d", seq, length_);

will expand to:

Logger.Debug("seq=%6d len=%4d", seq, length_);

that does the log. Or:

(void)sizeof("seq=%6d len=%4d", seq, length_);

that does absolutely nothing. It doesn't even evaluate the function arguments!!!

The trick is that the first version uses the comma as argument separator in a function call. In the second version, however, it is a unevaluated comma operator.

However, some compilers may give spurious warnings about unreachable code.

rodrigo
  • 94,151
  • 12
  • 143
  • 190
0

You could put the #ifdef inside the body of the individual functions. This avoid the code duplication issue in TooTone's answer.

Example:

void fastNative::Logger::Debug(std::string message, ...)
{
#ifdef ENABLE_LOGS
    // do the actual logging
#endif
}

If ENABLE_LOGS isn't defined, this function doesn't do anything. What I would suggest is you pass a const char* instead of std::string to these method. That way, if ENABLE_LOGS is not defined, you wouldn't have to rely on the compiler to not create redundant std::string objects.

Zeenobit
  • 4,954
  • 3
  • 34
  • 46