Is this preprocessor directive acceptable here?

Question

Having a singleton Logger class, I feel like writing Logger::GetInstance() everytime a print method is called is ugly. The only solution I can think of is #define. Is there anything better, or is this macro justified in this situation?

#include <iostream>
#include <fstream>

class Logger
{
public:

    static Logger& GetInstance();
    ~Logger();
    template <typename T>
    void   Print(const T &t);
    void   SetNewline(bool b);
    void   SetLogging(bool b);

private:

    Logger();
    Logger(const Logger&);
    void operator=(const Logger&);

    bool newline;
    bool log;
    std::ofstream file;
};

int main()
{
    #define logger Logger::GetInstance()
    logger.Print("Hello");
    //Logger::GetInstance().Print("Hello");
}

Answer 1

Two alternatives to Cheers and hth. - Alf's answer:

You could give the logger a name, if you need it more than once in a scope:

Logger& logger = Logger::GetInstance();

// ...

logger.Print("Hello, world");

Or you could make the logging methods static as well:

static void Logger::Print(const T &t) {
   Logger::GetInstance().Print(t);
}

and then call them statically:

Logger::Print("Hello world!");

You could argue that for the client it should not matter if there actually is an instance or not - the constructor is private so they can't create their own instance anyway. So whether the static methods create an instance or not should be none of their concern. In this case, having a public method called GetInstance actually exposes irrelevant implementation details.

Answer 2

note, since you're apparently defining a macro "locally", that macros do not respect scopes.

why not just define a function instead of a macro:

inline
auto logger() -> Logger& { return Logger::GetInstance(); }

then you can write just

logger().Print( "Hello" );