Singleton implementation

[Stewbond]

I'm playing with the idea of a singleton base class, but I'm having an issue with how to implement the GetInstance() function in the base class. Since I'm trying to make this ridiculously simple for the child, I'd like to handle that in the base.

class Singleton
{
private: 
  static Singleton* instance;
  Singleton() { Construct(); } // Private to avoid other instances
protected:
  virtual void Construct() = 0; ///< Initialization function used by children
public: 
  static Singleton* GetInstance() 
  {
    if (instance == 0)
      instance = new Singleton(); // Error!  I want this to be a child class
    return instance; 
  }
};

It would be easy to use like so:

class Hello : public Singleton
{
private:
  std::string hello;

  void Construct() { hello = "hello"; }
public: 
  std::string GetHello() const { return hello; }
};

Then the instance would be handled like so:
std::cout << Hello::GetInstance()->GetHello();

[Stewbond]

Here's my attempt at something nearly working:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35

#include <string> #include <iostream> template <class T> class Singleton { private: static Singleton* instance; protected: Singleton() { Construct(); } // Private to avoid other instances virtual void Construct() = 0; ///< Initialization function used by children public: static T* GetInstance() { if (instance == 0) instance = new T(); // Error! I want this to be a child class return instance; } }; Singleton* Singleton::instance = 0; class Hello : public Singleton<Hello> { private: std::string hello; void Construct() { hello = "hello"; } public: std::string GetHello() const { return hello; } }; int main() { std::cout << Hello::GetInstance()->GetHello(); }

||=== Build: Release in Test (compiler: GNU GCC Compiler) ===| /home/stew/SVN/Test/Test/main.cpp|32|error: specializing member ‘Singleton<Hello>::instance’ requires ‘template<>’ syntax| /home/stew/SVN/Test/Test/main.cpp||In instantiation of ‘static T* Singleton<T>::GetInstance() [with T = Hello]’:| /home/stew/SVN/Test/Test/main.cpp|16|error: invalid conversion from ‘Singleton<Hello>*’ to ‘Hello*’ [-fpermissive]| /home/stew/SVN/Test/Test/main.cpp||In instantiation of ‘Singleton<T>::Singleton() [with T = Hello]’:| ||=== Build failed: 2 error(s), 6 warning(s) (0 minute(s), 0 second(s)) ===| ||=== Run: Release in Test (compiler: GNU GCC Compiler) ===|

[Disch]

- line 7 should be static T* instance; (similar change on line 20 required). Either that, or static_cast instance to a T* on line 16.

- If the Child's ctor is private, the parent will need to be a friend in order to access it. Parents don't automatically have access to your private members. (EDIT: you weren't doing this.. nevermind)

- the virtual "Construct" member will not work because you cannot call virtual functions from a constructor or destructor (well you can... but it does not have the desired effect). Why are you bothering with this anyway? Why not just use a normal constructor? (EDIT: this actually makes more sense if you call construct after creating the object on line 16, rather than in the ctor. Though I would argue it should not be pure virtual.)

- Singletons are overrated and frequently overused. The only time I've ever really thought they were a good idea was for something like a resource manager... but even there it's debatable.

[Lowest0ne]

The Singleton instance pointer will be shared across all derivatives. When you get Hello working, Goodbye is going to be a problem:

#include <iostream>

struct Base
{
  static const char* word;

  Base( const char* w )
  {
    word = w;
  }

};

const char* Base::word = nullptr;

struct Child1 : public Base
{
  Child1( void ): Base( "Hello" ){}
};

struct Child2 : public Base
{
  Child2( void ): Base( "Goodbye" ){}
};


int main( void )
{
  Child1 one;
  Child2 two;

  std::cout << one.word << '\n';  // prints goodbye

  return 0;
}

It is a little funny to say that the problem is that you will only be able to have one instance of your singleton.

[Disch]

Lowest0ne:

Not if the base is a template. Base<Foo> and Base<Bar> are two different classes and therefore will have 2 different sets of static vars.

[Lowest0ne]

I did not know that, thanks. I also didn't see that you answered the question.

I played around with this myself and was unable to call the destructors:

#include <iostream>

template <typename T>
struct Base
{
  static T* the_instance;
  static T* instance( void )
  {
    if ( !the_instance ) the_instance = new T();

    return the_instance;
  }

  virtual ~Base( void )
  {
    delete the_instance;
    std::cout << "Base dtor\n";
  }

};

template< typename T> T* Base<T>::the_instance = nullptr;

struct Child : public Base< Child >
{
  ~Child()
  {
    std::cout << "Child Ctor\n";
  }
};


int main( void )
{
  Child::instance();
  return 0;
}

Edit:

If I remove the dynamic pointer, destructors are called correctly:

template < typename T>
struct Base
{
  static T* instance( void )
  {
    static T the_instance;
    return &the_instance;
  }
};

[10X9216C]

They also cause a lot of bloat, whenever Base<T>::instance() is called it has to do a check to see if it was initialized and it often inlines the constructor along with the check. It'd just be better to put it as a global object in a namespace.

[Stewbond]

I was making some global properties in my application. I thought a singleton would be nice to provide this information instead of passing it through a mess of constructors. A global instance would work, but I'd rather protect against multiple instances since there is no need for multiples.

A base-class is completely un-nessesary since I don't plan on using more than one, but I was trying it anyways to see how tough it would be.

I almost have it working thanks to the line 7 correction from Disch, but I'm having trouble calling the over-riden virtual function:

If virtual void Construct() is pure virtual, then I get
undefined reference to 'Singleton<Hello>::Construct()'

If it is defined as below, I get no error, but Singleton<T>::Construct() is called instead of Hello::Construct().

#include <string>
#include <iostream>

template <class T>
class Singleton
{
private:
  static T* instance;
protected:
  Singleton<T>() { Construct(); }
  virtual void Construct() { std::cout << "Should not happen";}
public:
  static T* GetInstance()
  {
    if (instance == 0)
      instance = new T();
    return instance;
  }
};

class Hello : public Singleton<Hello>
{
private:
  std::string hello;
  void Construct() { hello = "hello"; }
public:
  std::string GetHello() const { return hello; }
};

template <class T>
T* Singleton<T>::instance = 0;

int main()
{
    std::cout << Hello::GetInstance()->GetHello();
}

[LB]

I don't understand why you're making it so complicated?

http://coliru.stacked-crooked.com/a/44e36ddab9ac501c

template<typename T>
class Singleton
{
    friend T;
    Singleton() = default;
    virtual ~Singleton() = default;

public:
    static auto getInstance()
    -> T &
    {
        static T t;
        return t;
    }
};

class Test : public Singleton<Test>
{
    friend Singleton<Test>;
    Test(){}
};

I generally don't have any use for singletons though and actively try to avoid them.

[MikeyBoy]

I generally don't have any use for singletons though and actively try to avoid them.

Indeed. They're like global variables, in that they introduce unnecessary tight coupling within your code. And they're a pain in the ass when it comes to writing unit tests.

[ne555]

> but Singleton<T>::Construct() is called instead of Hello::Construct().
http://www.parashift.com/c++-faq/calling-virtuals-from-ctors.html

[dhayden]

I'm with MikeyBoy and LB. I rarely need singletons and if I do, I code them like this:

file s.h:
class MySingleton {
public:
   void method1();
   void method2();
   void method3();
};

file s.cpp:
#include "s.h"

static int member1;    // statics instead of private singleton members.
static int member2;
static string member3;

void MySingleton::method1()
{
   cout << "Look at me!  I'm all singly and stuff!\n";
}
// etc. 

You're coding a singleton. Only it isn't a singleton because it's a base class. I mean a not a base class but a template..... So there's really more than one.... My head hurts already.

Step back. What are you trying to accomplish? What is the simplest way to do it? That's the way that you should choose. I have a feeling that you're over-complicating things.

[LB]

dhayden, it's not a good idea to use static data members like that - they might not be initialized if other static data is being initialized based on your singleton.