Singleton question

[monkeycracks]

I've been studying the singleton design pattern lately, and there's something I don't quite understand.

In this example :

Code: Select all

#include <irrlicht.h>
#include <iostream>

class CCore {
private:
   CCore() {
   }
   static CCore* m_instance;
public:
   static CCore* Instance() {
      if(m_instance == 0)
      {
        m_instance = new CCore();
      }
      return m_instance;   
   }
   void displayHello() {
      printf("hello");
   }
};

CCore* CCore::m_instance = 0;

int main() {

   CCore * Core = CCore::Instance(); //OMG! A Singleton!!!111oneone... :O
   Core->displayHello();

}

Why is there no destructor and why is delete never called? I read somewhere that C++ automatically destroys static objects when the program ends. Can anyone verify this or give a different explanation?

[Dark_Kilauea]

That implementation will actually lead to a memory leak (although the OS might be nice enough to clean up for you). I like to use this, which in gcc will actually create thread safe code if it detects that multiple threads might touch it.

Code: Select all

class CCore 
{
private:
   CCore()
   {
   }
public:
   static CCore& Instance() 
   {
        static CCore theSingleInstance;
        return theSingleInstance;  
   }
   void displayHello()
   {
      printf("hello");
   }
};

The other bonus is that this way, you singleton will not memory leak, since dynamic allocation was not used.

[monkeycracks]

Fair enough.

Is there any reason to store the pointer rather than just use Class::Instance()->displayHello();?

[rogerborg]

Code: Select all

class CCore 
   static CCore& Instance() 
   {
        static CCore theSingleInstance;
        return theSingleInstance;  
   }

Since this doesn't allow any control over when theSingleInstance is created - which IMVHO is the only point of singletons, and not a very good point at that - how is it functionally different from a good old fashioned global?

I'm not saying that there's anything wrong with it, I'm just suggesting that the singleton pattern may be mutton dressed as lamb.

[monkeycracks]

Either way works for me, and I thought one of the points for using a singleton was to not have to pass pointers around.

For example, in the IrrWizard framework they pass the GameManager class around to each state, but in my framework I only have a GameManager class for init and release of core objects. I make an object (CRenderDevice for this example) a singleton, and just include CRenderDevice.h at the top of the SomeGameStateHere.h and from there I can call everything in CRenderDevice without having to pass anything around.

I don't care in what order the actual constructors/destructors are called because I don't do anything in them except set variables to 0 and similar things. I use init() and release() to perform initializations and cleanups, and I can control the order of that easily.

So I'm only using singletons for convenience. Maybe I shouldn't, maybe I should. Either way, it works.

[Murcho]

I've just setup my Irrlicht project with a Singleton, and from the research I've done, Singletons are great, but overuse of them is a bad idea. This article outlines the problems with them quite well.

http://www.softwarereality.com/design/singleton.jsp

The best thing I found was to put only the things that will be required everywhere in the project into Singletons, namely the Engine. I have a Singleton template class, which I use to inherit from when I want to make a singleton.

Code: Select all

// Singleton Template
// Taken from http://geeklab.wikidot.com/cpp-singleton-template
// Author : Rodrigo Varas

#ifndef SINGLETON_H
#define SINGLETON_H

template<class T>
class Singleton
{
private:
	// Static current instance
	static T* CurrentInstance;

public:
	// Current instance retriever
	static T* Current()
	{
		if (Singleton<T>::CurrentInstance == NULL)
		{
			Singleton<T>::CurrentInstance = new T;
		}
		return Singleton<T>::CurrentInstance;
	}

	//Delete current instance
	static void DeleteCurrent()
	{
		if (Singleton<T>::CurrentInstance != NULL)
		{
			delete Singleton<T>::CurrentInstance;
			Singleton<T>::CurrentInstance = NULL;
		}
	}
};

// Initialize the static member CurrentInstance
template<class T>
T* Singleton<T>::CurrentInstance = NULL;

#endif // SINGLETON_H

My Irrlicht singleton header looks like this

Code: Select all

#ifndef IRRSINGLETON_H
#define IRRSINGLETON_H

// IrrSingleton.h
// Created 22/11/08
// Author : James Murchison
// A singleton class for handling the Irrlicht Engine

#include "stdafx.h"
#include "Singleton.h"

class IrrSingleton : public Singleton<IrrSingleton>
{
public:
	IrrSingleton(void);
	~IrrSingleton(void);

	// Initializes the Irrlicht engine
	bool Init();
	
	// Called every frame to update
	void Update();

	// Functions for retrieving the main Irrlicht devices 
	IrrlichtDevice* GetDevice(){return m_pDevice;}
	video::IVideoDriver* GetVideoDriver(){return m_pDriver;}
	scene::ISceneManager* GetSceneManager(){return m_pSmgr;}
	gui::IGUIEnvironment* GetGUIEnvironment(){return m_pGUIEnv;}

	// Functions to create engine objects
	scene::ICameraSceneNode* CreateStandardCamera(core::vector3df _vPos, core::vector3df _vLookAt);
	scene::ICameraSceneNode* CreateFPSCamera(core::vector3df _vPos, core::vector3df _vLookAt);

private:
	// Irrlicht Device, used to create just about everything
	IrrlichtDevice *m_pDevice;

	// Video driver, interface for 2D/3D graphics functions
	video::IVideoDriver *m_pDriver;

	// Scene Manager, manages scene nodes, mesh resources, cameras, etc.
	scene::ISceneManager *m_pSmgr;

	// GUI Environment, factory and manager of all GUI elements
	gui::IGUIEnvironment *m_pGUIEnv;
};

// Definition of macro, makes calling the singleton a little easier
#define IrrEngine IrrSingleton::Current()

#endif // IRRSINGLETON_H

By doing this I have access to the engine where ever I am in my project, as long as I include the IrrSingleton header. If your code is structured properly, you shouldn't really have to make anything else a singleton.

Also the macro definition down the bottom of the header is nice, means I only type "IrrEngine" to get the singleton, rather than "IrrSingleton::Current()".

[rogerborg]

Either way works for me, and I thought one of the points for using a singleton was to not have to pass pointers around.

See also: global.

By doing this I have access to the engine where ever I am in my project, as long as I include the IrrSingleton header.

See also: global.

[Murcho]

A global will give me access to my class anywhere in the project, but it won't make sure there is only one instance of it, that is the point of a Singleton. I only require one instance of the engine, so having it in a Singleton ensures I don't accidentally create another instance of the engine.

Also, the singleton is only created the first time it is needed, so if the singleton class isn't used on a particular run through of a program, it isn't created, and resources aren't wasted. In my case it will always be used, it's the engine, but if for example a sound class was put into a singleton, and sound was disabled, then by having it in a singleton, the sound class wouldn't be loaded, and resources wouldn't be wasted.

So singletons are more than globals, they wouldn't be around if globals were the answer. It's just picking the right time to use them thats important.

[rogerborg]

We agree on the technicalities, just not on the implications of them.

Singletons are the plastic safety scissors of solutions. Personally I think that trusting your users to run with grown-up scissors is a fine way of teaching them a short, sharp lesson.

[Murcho]

Having looked at my code again, I could probably get away with having the Engine loaded in a global variable, but I find this solution a little more elegant.

I had this argument with a class mate a while ago, but he also thought global variables were the devil, so his code became extremely (and unnecessarily) complicated.

Having only been coding for about a year and a half, I probably shouldn't go head long into an argument about it. It is good to see both sides of the thought process though.

[monkeycracks]

@Murcho
-snip-
Just curious, doesn't having the constructor as public make it quite possible to just do new Engine();?

@rogerborg
I suppose using globals would be fine, but why would I use a global instead of a singleton? Less code i suppose, but it doesn't give me my plastic scissors that I so desire. Is there some advantage or do you just prefer globals over singletons?

[Acki]

Just curious, doesn't having the constructor as public make it quite possible to just do new Engine();?

but then you don't have a singleton...
http://en.wikipedia.org/wiki/Singleton_pattern

[monkeycracks]

That's what I was thinking. Murcho has the constructor and destructor both public in his IrrSingleton class. Me confused.

[rogerborg]

@rogerborg
I suppose using globals would be fine, but why would I use a global instead of a singleton? Less code i suppose,

There's your answer. If you're going to do more work, then IMO the onus is on you to demonstrate the benefits of it.

Singleton pros:
1) If written properly, enforces only one instance of a class.

2) Depending on how it is written, it delays instantiation of the instance until the point of use - although that can still produce problems with instantiation dependencies, and you also need to provide a cleanup method to the very users that you didn't trust to not multiply-instantiate the class.


Global pros:
While you were considering, explaining, designing, implementing, testing and providing workarounds for the above, I was writing actual production code using my global context.

Is pretty much the argument.

[jontan6]

i think prioritizing such design patterns is more popular with java people