Correct way to implement Singleton

Correct way to implement Singleton
0.0 0

#21

Hey @qwpeoriu Thanks for the links.

By the following code,

struct Z {
    Z(long long);     // can initialize with an long long         
    Z(long) = delete; // but not anything less
};

I am not sure how will this help to delete a instance of the singleton class.

I am seeing that whenever the app is closed, direct is called to CC_SAFE_RELEASE to delete all the nodes and related memory. But not sure how does it happen for non-node classes like singleton class.

Hey @slackmoehrle
Plz see my last post…

I didn’t really understand by:

Thanks :slight_smile:


#22

The “= delete” syntax tells the compiler to ignore a function/method, I was suggesting it as a way to block the copy constructor and assignment operator for your singletons (a good practice). It doesn’t have anything to do with destructors or deleting an instance. When and how to delete your singleton is a very good question though, so I will answer that in a later post (I don’t want to address 2 topics in this one post in order to avoid confusion).

The old way to block the copy constructor / assignment operator is like in your example, you make them private in the header file and provide no implementation in the CPP file, like this:

OLD C++ WAY

// In your header:
class MySingleton {
private:
	MySingleton(const MySingleton& arg); // Copy constructor
	MySingleton& operator=(const MySingleton& arg); // Assignment operator
}

In C++11 you can do this, and to be completely thorough here are all of the methods to block:

NEW C++11 WAY

// In your header
class MySingleton {
public:
	MySingleton(const MySingleton& arg) = delete; // Copy constructor
	MySingleton(const MySingleton&& arg) = delete;  // Move constructor
	MySingleton& operator=(const MySingleton& arg) = delete; // Assignment operator
	MySingleton& operator=(const MySingleton&& arg) = delete; // Move operator
}

I suppose this is nitpicking but I prefer the new C++ way, because my intention is clear that I want the copy constructor and assignment operators blocked.


#23

I know you directed these questions to @slackmoehrle, so hopefully he doesn’t mind if I provide my opinion on them.

You need a destructor if your singleton allocates class members with new and needs to delete them, or it keeps file handles or network connections open and needs to close them, or it calls retain on CC2D objects and needs to release them, etc …

Really though, only you can figure out if your singleton needs a destructor, because only the person who wrote the code knows if they have a resource that needs to be released at the end of the singleton’s life.

As far as when to call the destructor for your singletons, I specifically do not allocate my singletons on the heap (using new) for this reason. My C++ code almost never calls delete explicitly. I declare variables at the automatic scope (local variable, class member, etc …) wherever I can, and if I must instantiate something with new, the first thing I do is manage it with a smart pointer (and the smart pointer will be declared at the automatic scope).

Don’t manage things when you don’t have to, you’ll just give yourself a headache chasing down bugs when you get something wrong.

If you see my singleton pattern in one of my earlier posts above, I define the singleton as a static local variable. C++ will call that static local variable’s destructor when the program exits (meaning my singleton will exist for the lifetime of the program, which is the behavior I want), there is no need for me to ever do so explicitly. Here is a good explanation on the lifetime of a C++ static variable:

Here is my full C++ singleton pattern:

Header File:

class MySingleton {
public:
	// Singleton getter
	static MySingleton& getInstance();

	// Methods to block
	MySingleton(const MySingleton& arg) = delete; // Copy constructor
	MySingleton(const MySingleton&& arg) = delete;  // Move constructor
	MySingleton& operator=(const MySingleton& arg) = delete; // Assignment operator
	MySingleton& operator=(const MySingleton&& arg) = delete; // Move operator
	
private:
	MySingleton();
	virtual ~MySingleton();
}

C++ File:

MySingleton& MySingleton::getInstance() {
	static MySingleton instance;
	return instance;
}

MySingleton::MySingleton() {
	// Constructor implementation
}

MySingleton::~MySingleton() {
	// Destructor implementation
}

This pattern adheres to good RAII practices, and understanding RAII is crucial to writing good C++ code. That is why I declare just about everything I can at the automatic scope, and use smart pointers for everything I can’t. Here are some good resources for understand RAII:

http://en.cppreference.com/w/cpp/language/raii


#24

This answer deserves an award. I really like the c++11 way of implementing a singleton. I’m going to test this out.


#25

@qwpeoriu
Thanks a lot for elaborate explanations. :slight_smile:
I think I’ve cleared my doubts. Thanks :slight_smile:

So, I also switched to completely C++11 style as what you suggested, works fine.

In case, I had to use new for the instance of singleton then at what point of program shall I be calling destructor (delete instance) if singleton remains for the entire life of the program(game). Or what happens in case the user force quits the program… In that case, does OS handles the things automatically?

Just wanted to know if your other class members(variables) are all static because you’re not allocating memory to your singleton instance or it need not to be?
(I’m accessing non-static class members right now… working fine though.)

Thanks :slight_smile:


#26

I usually implement singleton like this:

  • create a template singleton
    Singleton.h
#ifndef __SINGLETON_H__
#define __SINGLETON_H__

template <typename T>
class Singleton
{
public:
	inline static T* getInstance();
	inline static void release();
private:
	static T* t;
};

template <typename T>
inline T* Singleton<T>::getInstance()
{
	if (!t)
	{
		t = new T;
	}
	return t;
}

template<typename T>
inline void Singleton<T>::release()
{
	if (t)
	{
		delete t;
		t = 0;
	}
}

template <typename T> 
T* Singleton<T>::t = 0;

#endif // __SINGLE_H__
  • implement singleton, for example GameManager
    GameManager.h
#include "singleton.h"
class GameManager : public Singleton<GameManager>
{
public:
    
    GameManager();

  // Game Play
  int getGameScore();
  void setGameScore(int latestScore);

private:
  int gameScore;
    
};

GameManager.cpp

#include "cocos2d.h"
#include "GameManager.h"

USING_NS_CC;

void GameManager::GameManager()
{
    this->gameScore = 1;
}

int GameManager::getGameScore()
{
    CCLog("\n\n\nGETTING GAME SCORE: %d\n\n\n", this->gameScore);
    return this->gameScore;
}

void GameManager::setGameScore(int latestScore)
{
    CCLog("\n\n\nSETTING GAME SCORE: %d\n\n\n", latestScore);
    this->gameScore = latestScore;
}

using

  GameManager::getInstance()->getGameScore();
   GameManager::getInstance()->setGameScore(50);

Remeber release singleton at the end of game (in destructor of AppDelegate).
AppDelegate.cpp

AppDelegate::~AppDelegate() 
{
    GameManager::release();
}

#27

Hey @doanhieuthien

Thanks a lot of detailed reply. Your solution is good.

I was reading somewhere that singletons objects (non static) created might not be thread safe. So, if we’re sure about how we are handling our objects, we’re fine to use it. Since, I’m not too techy in C++, it’s ok for me to use this for simple games but may not be if the complexity of my future game is more and I might be unaware when it happens.

(Finally been answered by someone which I initially have been asking :stuck_out_tongue: :D)

As always, thanks for a good explanation and code. :slight_smile:


#28

Some of you guys not using smart pointers where you can, gross :confounded: :wink:


#29

@Jgod. Teach a man to fish…


#30

Some of you guys insist on over-engineering your simple match-3 game clone instead of just using global variables directly, gross :wink:


#32

I didn’t notice how good @qwpeoriu’s answer was the first time around. Well-written and covers everything I thought I was adding.


#33

I know this comment might not be for me but initially I was using global variables through a namespace but I had to switch to singleton because I had around 100 variables which I didn’t want to allocate memory to when it’s not needed.


#34

Use UserDefault then. :smiley:

Or don’t worry about the memory allocated since I doubt the size of the global state is all that large compared to the game’s Texture resources. Or store large states in global pointers (or unique/shared_ptr) and allocate as necessary.

Anyway, this thread is on correct singletons.

I use the cocos2d method just because there’s no thought involved and I came from c2d-ObjC.
I often use a static class with static members to load/save prefs as a cache for UserDefault.
I’ll consider qwperoiu’s version going forward, but agree it’s generally smart to avoid global state.

Could also serialize in/out any info that needs to exist across a scene transition, or pass the World state to each new scene (directly or using Dep.Inject.). Or store it in a temporary/permanently global using your game’s cocos2d::App instance.

p.s. I tend toward facetious remarks and partial-sarcasm when i want to play devil’s advocate, but don’t want to write a real response of substance.


#35

Agreed, if we can handle brutal compiler errors all day we can handle a little banter :slight_smile: