CODEWITHSUNDEEP
c++singletonconstantslogging
knight666
knight666

Reputation:

How can I make an instance of my singleton const?

I'm trying to make a wrapper for Ogre (an open source 3D engine) 's standard logging class. I want it to have the same syntax as std::cerr, and also output to cerr when running on Linux. Here's what I have:

#ifndef _LOGGER_H_
#define _LOGGER_H_

#ifndef _XSTRING_
    #include <xstring>
#endif

#ifndef __LogManager_H__
    #include "OgreLogManager.h"
#endif

class Logger
{

public:

    static Logger* m_Instance;
    static Logger* getInstance() { return m_Instance; }
    static const Logger& getInstanceConst() { return *m_Instance; }

    Logger& operator << (const std::string& a_Message)
    {
        m_Log.append(a_Message);
        _CheckNewLine();
        return *m_Instance;
    }

    Logger& operator << (const char* a_Message)
    {
    m_Log += a_Message;
    _CheckNewLine();
    return *m_Instance;
}

private:

    std::string m_Log;

    Logger() 
    {
        m_Log = "";
    }

    void _CheckNewLine()
    {
        if (m_Log.at(m_Log.size() - 1) == '\n')
        {   
            Ogre::LogManager::getSingleton().logMessage(m_Log);

#if OGRE_PLATFORM != PLATFORM_WIN32 && OGRE_PLATFORM != OGRE_PLATFORM_WIN32

            std::cerr << m_Log;

#endif

            m_Log.clear();
        }
    }
};

#endif

Now, this works fine, and this singleton is instanced in a .cpp:

#include "logger.h"

Logger* Logger::m_Instance = new Logger();

The problem comes when I want to use the singleton in multiple headers. I instantiate it in game3d.h, which is included by pretty much all the headers like this:

Logger awesomelogger = Logger::getInstance();

Unfortunately, this gives multiple errors about headers trying to redeclare awesomelogger.

I want to make it a const, which would make that go away, but that introduces new errors. This is what I tried:

friend Logger& operator << (const Logger& a_Logger, const std::string& a_Message)
{
    a_Logger.m_Log.append(a_Message);
    a_Logger._CheckNewLine();
    return *m_Instance;
}

My question is: how can I make an instance of this class constant OR how can I rewrite this class but still be able to do awesomelogger << "output" << s_Stuff << "\n";

Upvotes: 0

Views: 1858

Answers (6)

Jeff Leonard
Jeff Leonard

Reputation: 3294

It sounds from this:

The problem comes when I want to use the singleton in multiple headers. I instantiate it in game3d.h, which is included by pretty much all the headers like this:

 Logger awesomelogger = Logger::getInstance();

That you are creating a new instance of awesomelogger in every module that includes game3d.h and since this declaration has external linkage, these names will collide at link time.

See Linkage in Names with File Scope for an explanation of C++ linkage rules.

A better solution is to dispense with creating the awesomelogger variable and just call Logger::getInstance() directly wherever you need it.

Upvotes: 0

Loki Astari
Loki Astari

Reputation: 264631

The classic singelton pattern looks like this:

#ifndef  THORS_ANVIL_MY_LOGGER_H
#define  THORS_ANVIL_MY_LOGGER_H

class MyLogger
{
  private:
   // Private Constructor
    MyLogger();
   // Stop the compiler generating methods of copy the object
    MyLogger(MyLogger const& copy);           // Not Implemented.
    MyLogger& operator=(MyLogger const& copy); // Not Implemented

  public:
    static MyLogger& getInstance()
    {
        // The only instance
        // Guaranteed to be lazy initialized
        // Guaranteed that it will be destroyed correctly
        static MyLogger instance;
        return instance;
    }
    template<typename T>
    MyLogger& operator<<(T const& data)
    {
          // LOG
          return *this;
    }
};

// continued below.

Now we have the basic pattern out the way. You need what looks like a global variable to access it. The simplest way is to cheat and not have a global, but use a local reference.

So in the header file along with your Logger class definition add the following.

namespace
{
    MyLogger&   logger = MyLogger::getInstance();
}
#endif

This declares a file local variable in every compilation unit that includes the header. That has been initialized to refer to instance of the logger that is a singelton. Because you need to include the file before you can use it will always be declared before any usage and thus guranteed to be initialized in the correct order.

In main file:

#include "MyLogger.h"
int main()
{
    logger << "Plop";
}

Upvotes: 0

Sam
Sam

Reputation: 2201

Generally speaking too, your instance of Logger "m_Instance" should be private. And your function "GetLogger()" should check if "m_Instance" has not been instantiated (if not instantiate it) then return m_Instance.

Upvotes: 0

stribika
stribika

Reputation: 3176

Your get_instance method returns Logger* not Logger.

Upvotes: 1

bohdan_trotsenko
bohdan_trotsenko

Reputation: 5367

make a const pointer to a const object.

static const Logger * const m_Instance;

instead of

static Logger* m_Instance;

You'll need a lot of respective corrections in the class too.

Upvotes: 0

anon
anon

Reputation:

Not to do with your question, but ote that names such as _LOGGER_H_ and __LogManager_H__ are reserverd in C++ - you are not allowed to use them in your own code. If you don't understand the rules regarding underscores at the start of names (or double underscores anywhere), then don't use them.

Now, regarding your question, a logger obviously isn't const. The classic way to provide access to a singleton is some variation on this:

static Logger* getInstance() { 
    static Logger logger;
    return & logger; 
}

Upvotes: 1

Related Questions