Jump to content
  • Advertisement
Sign in to follow this  
schupf

Singleton Design

This topic is 2610 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello,

I have some singletons in my program. For example an App singleton (controls game loop etc) and a Logger and MemoryManager singleton. I guess it is preferable to control the lifetime of the singletons, for example I want to be sure logging in the App destructor is possible. Therefore I thought about this design: The App singleton is kinda like a singleton manager. App is created first, creates all other singletons and is also responsible for destroying them.

The code would roughly look like this:

// Main Singleton
class App {
private:
App();
App(const App& rhs);
~App();
App& operator=(const App& rhs);

static App* instance;

public:
static App* createInstance() {
if(instance == NULL)
instance = new App();
return instance;
}

static App* getInstance() {
assert(instance);
return instance;
}

static void destroyInstance() {
if(instance)
delete instance;
}

private:
App() {
// Andere Singletons erzeugen:
Logger::createInstance("MyLogger");
MemoryManager::createInstance();
...
}

~App() {
// Andere Singletons zerstören:
Logger::destroyInstance();
MemoryManager::destroyInstance();
...
};

//Usage:
int main() {
App* a = App::createInstance();
// Now App and all other singletons are usable
a->foo();
...

App::destroyInstance();
}

Do you think this will work? Or do you see any big problems/design flaws?
Do you think the design is good?

PS: I am NOT using multithreading.

Share this post


Link to post
Share on other sites
Advertisement
Looks fine (its better than the static singleton approach). I do prefer to have somehtign actually own it and not have it created on first use and destroyed in some random order at exit. I would just use the constructor/destructor instead of create instance/destroy instance. Though perhaps your idea is more explicit in describing whats going on. I would definatly make it fail outright if you try to create an instance when one already exists (rather than just returning the current instance), otherwise, whoever tried creating it will assume they now own it and are responsible for destroying it too.

static App *instance = NULL;

class App
{
public:
App()
{
assert(instance == NULL); // Throw some error, force it to crash so the programmer only ever calls it once by design.
instance = this;
}
~App()
{
assert(instance == this);
instance = NULL;
}
static App* GetInstance()
{
// Throw an error if no instance exists
return instance;
}
};


I would first think about why you need to make this App class globally accessable though.

Share this post


Link to post
Share on other sites
Note that your code becomes a lot simpler when not using singletons here:class App {
public:
App() : m_Logger("MyLogger"), m_Memory() { m_Logger.Log("Starting up"); }
~App() : { m_Logger.Log("Shutting down"); }
private:
Logger m_Logger;
MemoryManager m_Memory;
};

//Usage:
int main() {
App a;
// Now App and all app's members are usable
a.foo();
...
}
No concerns about order of creation/destruction either...

What does all the added complexity of using the singleton pattern give you here (besides the disadvantage of turning things into global variables)?

Share this post


Link to post
Share on other sites
Thanks for your responses.

@Nanoha: Just out of curiosity: Why do you use a module static global variable: static App *instance = NULL; ?
Is that intentional or did you mean a class variable: App* App::instance = NULL;

@Hodgman: You are right and actually I do not like singletons. But in this case it makes sense because more than one App instance makes no sense (app controls the window message loop and there only can be one message loop etc.).
Plus the logger is needed everywhere and it would be very tedious to pass through a logger pointer to all the classes that might need logging.

Share this post


Link to post
Share on other sites

Thanks for your responses.

@Nanoha: Just out of curiosity: Why do you use a module static global variable: static App *instance = NULL; ?
Is that intentional or did you mean a class variable: App* App::instance = NULL;



I did mean static, that bit would usually go in the implementation(cpp) and not the header. It could go in the class definition.

If you want the logger to be globally accessable, then make just the logger globally accessable (and not the App). If you want to enforce the App as a singleton, then just have a flag saying if an instance has been created and force it to crash/throw an exception.

class App
{
private:
static bool instanceExists = false;
public:
App()
{
// Check instance doesn't exist
if(instanceExists) { throw somehting; }
instanceExists = true;
}
~App()
{
instanceExists = false;
}
};

Share this post


Link to post
Share on other sites

But in this case it makes sense because more than one App instance makes no sense (app controls the window message loop and there only can be one message loop etc.).

An alternative would be to create a single App object, and then pass it forward to any other objects that need access to it.

Plus the logger is needed everywhere and it would be very tedious to pass through a logger pointer to all the classes that might need logging.[/quote]
There are alternatives to singletons here as well. For example, you could make the logger a simple global object.

Share this post


Link to post
Share on other sites
C++ singletons seem really silly to me. Why not just use a namespace?:
App::Log(const std::string& s)
Or create a single instance of the class, using what you would normally think of the class name:
class App_
{
void Log(const std::string& s);
};
App_* App = new App_;

Why all these drastic measures to try to conform to a paradigm C++ doesn't support?

Share this post


Link to post
Share on other sites

Why all these drastic measures to try to conform to a paradigm C++ doesn't support?


The most important reason whenever I use it: not having it created at some random point before main.

A somewhat constructed example.


class AutoRegister
{
AutoRegister() { globalMap[id] = this; }
};


AutoRegister globalObject;


This can crash horribly, if globalMap is in some other .cpp, since you have no control if globalObject or globalMap is created first. In one case you're lucky, in the other you will waste a lot of time wondering why accessing the map will fail badly.


map<x,x>& registryMap()
{
static map<x,x> instance;
return instance;
}


Now it can't crash, because calling "registryMap()[id] = this" will make sure that the map is instanced before you actually access it for the first time.

Personally I think it partially defeats the purpose of a singleton, if it has to be explicitely created and destroyed. Sure, you still make sure there can be one and only one (kind of), but all the automatism and safety is ripped right out of it.

Obviously, as long as you never ever access a global object from another global objects constructor, this shouldn't become an issue. A common example however would be static registration with a factory.

Making the logger a global object could suffer the same fate, if object constructors try to use the logger which tends to happen in the kind of classes that are tempting to make global as well (class MyEngine, etc.)

In this simple case you might notice that I don't bother wrapping the map in a dummy class, just to make some static "getInstance()" function.

Share this post


Link to post
Share on other sites
class AutoRegister
{
AutoRegister() { globalMap[id] = this; }
};
AutoRegister globalObject;
Going off topic, but when you least expect it, the linker is going to dead-strip that code and it's not going to make it into the final EXE, leaving you wondering why auto-registration has suddenly stopped working for certain classes...

Running code before main is evil.

Share this post


Link to post
Share on other sites
Its me again:)

I changed my design a little bit. App is not a singleton any longer. It is a normal class which just can be instantiated one time (but no global static access functions are available). Lets assume all my Singletons (Logger, MemoryManager) are Meyer singletons with a static local variable, e.g.:

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

App::App() {
Logger::getInstance().log("App created.");
}

// App destructor:
App::~App() {
Logger.getInstance().log("App destroyed.");
}

int main() {
App* app = new App(..);
app->doStuff();
delete app;
}

As you can see the Logger is used the first time in the App Ctor. Now I wonder WHEN the logger (and all other singletons) get destroyed.
Is it safe to use the logger in the App destructor and the destructor of all other singletons?

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!