0

I'm trying to write a log class with threadsafe practice in c++. Now the thing is, I want to make the call to each log line very simple. I can go with static class way like below:

//Calling this in some GUI initialization point of application
CLogger::InitLogger("LogFile.log");

//Calling below from any class just adding the header to the cpp
CLogger::Log("Some informational text goes here");

Now this doesn't follow the OOP so would like to go with a singleton class.

//Singleton log class
class CLogWrite
{
public:
  static CLogWrite* GetInstance();

private:
  static CLogWrite *pInstance;
  void Log();
};

CLogWrite* CLogWrite::GetInstance()
{
  if(pInstance != NULL)
  {
    pInstance = new CLogWrite;
  }
  return pInstance;
}

void CLogWrite::Log()
{
  //threadSafe lock

  //write 'logText' to file here

  //threadSafe unlock
}

Now the thing is, if I write above class and call CLogWriter::GetInstance() inside my GUI class init function like below:

//single logger instance for my app
CLogWriter *mLogInstance;
mLogInstance = CLogWriter::GetInstance()

I need to pass 'mLogInstance' variable to each and every class inside my project from where I want to write a log. But I don't wanna do that.

What would be best approach?

hypheni
  • 756
  • 3
  • 17
  • 37
  • why cant you declare as global variable, because in our application we were using as global variable – VINOTH ENERGETIC Apr 11 '15 at 11:43
  • "variable to each and every class" . what does this means. – VINOTH ENERGETIC Apr 11 '15 at 11:45
  • 2
    Why reinvent the wheel? You can use an existing logging library like [Boost.log](http://www.boost.org/libs/log/doc/html/index.html) or [glog](https://github.com/google/glog). – Kerrek SB Apr 11 '15 at 11:47
  • Global variable is the quickest thing came to my mind. But that also breaking the rules of OOP. Variable to each and every class - this means. In an application project I have several class. Say one main GUI class along with several worker class. i.e. CFileReadWrite, CSocketDataOperation, etc. – hypheni Apr 11 '15 at 11:47
  • @glog. – Kerrek.. Well its a general concern of mine. I don't wanna use ready made log class as these codes needs to be run on various platforms. – hypheni Apr 11 '15 at 11:49

2 Answers2

2

Try this:

class Singleton
{
public:
    static Singleton& getInstance()
    {
        static Singleton instance;
        return instance;
    }

    Singleton(Singleton const&) = delete;
    void operator=(Singleton const&) = delete;

private:
    Singleton() {};        
};
  • Can you please explain this. – VINOTH ENERGETIC Apr 11 '15 at 12:08
  • Could you please explain a bit. – hypheni Apr 11 '15 at 12:09
  • @PLearner It's well known as [_Scott Meyer's Singleton idiom_](http://stackoverflow.com/questions/25173716/player-obj-error-lnk2001-unresolved-external-symbol-private-static-class-in/25173785?s=2|0.3768#25173785) – πάντα ῥεῖ Apr 11 '15 at 12:12
  • If you call getInstance the first time, instance will be lazy initialized. The Constructor and Copy Constructor is private, so there can be only one instance. –  Apr 11 '15 at 12:14
  • what is the usage of 'void operator=(Singleton const&) = delete;' ? – hypheni Apr 11 '15 at 12:17
  • @PLearner These remove the declarations of copy constructor and assignment operator. – πάντα ῥεῖ Apr 11 '15 at 12:18
  • He wanted thread-safe code. First of all, there was no word about C++ 11, and only this and above standards guarantee initialization of local statics to be thread-safe. Secondly, some compilers still don't support this feature (VC++ 11 to be an example). – Mateusz Grzejek Apr 11 '15 at 12:28
  • I want my code to be supported on old compilers. Not C++ 11. Currently targetting both VC++ 2005 and 2012. – hypheni Apr 11 '15 at 12:32
  • 1
    If you're going to delete copy constructor and copy assignment operator they should be public, not private. If you don't want to use the C++11 delete keyword and syntax, then they should be private. With your example, if a use of either of the two functions is detected by the compiler, it will complain about the use of a private function rather than use of a deleted one, which is of course less precise. – tweej Apr 11 '15 at 12:58
  • @PLearner Then don't use `= delete` syntax presented above, since it's a C++ 11 feature and it's not supported even in VS2012. Make copy constructor and assignment operator private and don't provide definition for them. – Mateusz Grzejek Apr 12 '15 at 11:11
  • @Mateusz Grzejek.. Can you show me an example or guide me with my below posted code. – hypheni Apr 13 '15 at 14:46
1

One of my favourite techniques in C++ is so-called CRTP, which can be used to implement singleton logic. Look at this code:

template <class Type>
class Singleton
{
private:
    static std::unique_ptr<Type>    _holder_ptr;
    static std::mutex               _mutex;

public:
    static Type& GetSingleton()
    {
        if(!_holder_ptr)
            _create_instance();

        return *(_holder_ptr.get());
    }

protected:
    static void _create_instance()
    {
        _mutex.lock();

        if(!_holder_ptr)
            _holder_ptr.reset(new Type());

        _mutex.unlock();
    }
};

Now, you can use this class to define "singletons":

class Log : public Singleton<Log>
{
    //content
};

and use it:

Log& global_log = Log::GetSingleton();
//do something

This is version for C++ 11. For older compilers, replace std::mutex with platform-dependent synchronizing primitive. On Windows, you may want to use CRITICAL_SECTION. Sample usage: here.

std::unique_ptr was also introduced in C++ 11, so you may either replace it with std::auto_ptr (deprecated in C++ 11), use smart pointer from Boost library or create your own solution (which is not hard, but reinventing the wheel is not a good idea).

Note, that this Singleton does not forbid to create additional instances of Log - and it should not! There are two common scenarios:

  • You allow users to create additional instances of Log, but at the same time demand, that at least one such object must exist. Then, simple deriving from Singleton is all what you need.
  • You need Log to be true "Singleton", that is: only one, shared instance can exist - rest of code must use this instance, so everything can work as expected. In this case, use = delete syntax to remove default constructor, copy constructor and assignment operator (or simply declare them as protected or private, depending on compiler support for = delete syntax).

Also, remember, that if you want to hold shared instance as pointer, you need to use some kind of smart pointers, as presented above. You cannot use plain pointers like in your original solution:

CLogWrite* CLogWrite::GetInstance()
{
    if(pInstance != NULL)
    {
        pInstance = new CLogWrite;
    }
    return pInstance;
}

Where and when pInstance would be destroyed? How?

Mateusz Grzejek
  • 11,698
  • 3
  • 32
  • 49
  • Thanks for the neat code and its explanation. Now coming to 'Log& global_log = Log::GetSingleton();' - this thing either needs to be a class member or global variable. Isn't it? – hypheni Apr 14 '15 at 08:46
  • What do you mean? All you need to do is to call `GetSingleton()` every time you need to retrieve global instance of singleton class. – Mateusz Grzejek Apr 14 '15 at 09:40
  • Oh yes. I got it now. – hypheni Apr 14 '15 at 09:47