making a class only get constructed once

hello.

I'm trying to make a CWorld class that can only be constructed once, now Elysia told me the approach yesterday, still I forgot all the details so I've been mindless hacking for sometime, with only help from intellisense and some from cprogramming.com, still I ain't getting it right...

Code:

---


class CWorld
{
private:
        hgeVector m_hVec_ScreenSize;

        float m_Gravity;

        boost::array<std::string, 1> m_str_mapNames;
        void SetMapNames();

        CWorld(const float gravity_);

public:

        hgePolygonMap* m_map;

        friend class CWorld_Fac;

        // HGE needed functions e.g. the creation of the world...
        inline void SetStates();
        static bool FrameFunc();
        static bool RenderFunc();

};

class CWorld_Fac {

public:

        CWorld_Fac::CWorld_Fac(const float gravity_) {

                static CWorld world_(gravity_);
        }
};

---

Those are the classes I have, now I'm supposed to be able to create many CWorld_Fac classes, still it will always lead to the same CWorld class, since it's static, also I have this code in main()

Code:

---

world = new CWorld_Fac(0.5f);

---

now this should be having the right info, and thus making CWorld get the right info first time...
here is the call that fails

Code:

---

CWorld_Fac m_world;

// this fails
CCharacter(std::string entClass_) : CEntity(HEL_MIDDLE2, false), m_world(0) { m_world.m_map->RegisterEntityClass(entClass_, this); }

---

now here I set m_world argument to 0, this shouldn't change anything since the CWorld constructor was static in CWorld_fac, right?

Hope I was able to explain correctly, and thanks in advance!

For the record, this pattern is called a singleton. Makes google easier ;)

You should create a class with three private functions, that need not have a body, only a definition:
- The copy constructor
- The operator= with as argument the object itself
- The constructor (ok, this should have a body)

Then, a single, static member function, eg. called getInstance, which returns a reference to the object itself. Then do something similar to this:

Code:

---

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

Singleton &Singleton::getInstance()
{
  static Singleton object;
  return object;
}

---

The object will be constructed once. To get a reference to the object, use Singleton::getInstance(). You can use it directly, eg:
Singleton::getInstance().doSomething();

Well, I just got information that what I'm trying to do has already been done -.-
We're using a SDK from another guy, hence we don't know ALL the code :S

I don't see a reason why World should be constructed just once (in a game, why couldn't there be many worlds?).

If you want one instance, don't create more.

Singleton is often considered an antipattern: it basically encourages you to use global state while thinking what you are doing is OOP.

------

Perhaps you could add a static counter to the class that you increment in the constructor and assert or throw if the counter gets above 1?

we agreed on only one world, and it holds gravity and such, also for other classes to use it, they have to construct it again or pass it through argument (which I don't want)

Quote:

---

Originally Posted by Akkernight

pass it through argument (which I don't want)

---

Why not? After all, although you may decide up front that you only want one world, if you keep your design reusable then maybe even if the requirements change later it would not be such a hassle to accommodate the changes.

My solution to this is an object factory:

Code:

---

#ifndef OBJFACTORY_174815_H
#define OBJFACTORY_174815_H

template<typename T>
class ObjFactory
{
public:
        static T* Create()
        {
                assert(m_Instances == 0); // Allow only one instance of the object
                if (m_Instances >= 1)
                        return NULL;
                T* p = new T;
                m_Instances++;
                return p;
        }

private:
        static int m_Instances;
};

#endif // OBJFACTORY_174815_H

---

No singleton, no global and will allow the construction of only one object.
Of course, the object being constructed needs a private constructor and make the object factory a friend.

Yikes!

So... you just terminate the program if you accidentally try to create a second one? And only in debug mode? But you allow more than one to be created in release mode?

I think you need a new solution...

Anyway, as anon said, if you only want one instance, don't create more than one.

If you are only using the singleton pattern to avoid passing parameters... you fail.

Soma

It's not to hinder; it's to catch mistakes.
I could easily make it throw an exception, but at this point, I didn't think it was necessary.
Or is it necessary in your opinion? Or perhaps simply returning NULL?
Well, returning NULL doesn't hurt, so let's add it as a good measure.

Meh. Not such a good design, since you can't rely on it returning NULL, it at least it catches your mistakes in Debug mode.

None of the above...

I was just making a few points:

1): You should never use `assert' for checks that should always be performed.
2): An object capable of being instantiated more than once is never a singleton--regardless of what people might like to call it.
3): "Crash and Burn" is not a good strategy to handle events very likely to occur in the course of normal program execution.

Soma

Quote:

---

Originally Posted by phantomotap

None of the above...

I was just making a few points:

1): You should never use `assert' for checks that should always be performed.
2): An object capable of being instantiated more than once is never a singleton--regardless of what people might like to call it.
3): "Crash and Burn" is not a good strategy to handle events very likely to occur in the course of normal program execution.

Soma

---

1) Very true, but that is not the main purpose.
2) o_O I wouldn't call this a singleton at all anyway.
3) Absolutely, but this design was meant to catch mistakes in trying to instantiate two or more objects of the same type, not hinder them. Therefore, to the first point, I used an assert and not an if to check if there are 2 or more instantiations.

I could make one that checks each time you make one and fail if it already has made one... But I didn't feel it necessary. Is it necessary? I can't say.

Quote:

---

Originally Posted by Elysia

It's not to hinder; it's to catch mistakes.
I could easily make it throw an exception, but at this point, I didn't think it was necessary.
Or is it necessary in your opinion? Or perhaps simply returning NULL?
Well, returning NULL doesn't hurt, so let's add it as a good measure.

Meh. Not such a good design, since you can't rely on it returning NULL, it at least it catches your mistakes in Debug mode.

---

I wouldn't throw an exception, since you could leak the memory of the allowed instance if you unwind the stack too far. In my opinion, I think the best idea is to just make one instance, rather than rely on some boilerplate or your idea that returns zombie objects when you don't have the presence of mind to restrict yourself. Just check the code.

Making the class noncopyable (isn't there something like Boost::noncopyable?) takes care of the concern of the instance being used as a function argument to a large extent.

Then we're back to the first design - an object factory which can catch your mistakes and save you time.
Your suggestion about non-copyable is good too.
Yeah, boost has a noncopyable class that you can inherit from to disable copying.

Show me one coder who has accidentally created more instances of something as big as CWorld than he wanted and didn't even notice the code was malfunctioning and needed correcting.

It's really all about making an instance global in this case.

I know of one coder whose name I'm not about to give out, who actually created another instance of a singleton.
I had to make the constructor private and use friends to catch that mistake.

Quote:

---

I had to make the constructor private and use friends to catch that mistake.

---

It's not really a good singleton implementation if the constructor is public....

Of course not. But the singleton factory also had a public constructor. I didn't think anyone was stupid enough to actually create one o_O
It just shows - take nothing for granted.

It was laserlight wasn't it?

Soma

The singleton pattern is a known pattern for this and as such I would certainly use it. Any other code to address this is probably fraught with issues. The only time I've had problems with singletons is with DLLs and in multi-threaded apps. Both issues can be overcome though quite easily.

Writing some off the wall code to accomplish what a singleton does, does not seem to me to be a good practice.

The idea behind Elysia's approach is that the class has a single responsibility (doing its thing) and the factory has another (ensuring how many are created). The main difference is that you get the uniqueness guarantee (what singleton is about but which is hardly ever essential) without making a global instance (why people, including OP, mostly want to use singletons).

So the singleton pattern ensures that you have only one instance. However, as with global, after calling GetInstance() you have really no idea whether it is OK to use it the way you do. May-be the object has been previously put into a specific state, so that only certain actions are valid now? Singleton may solve a single problem but may have the tendency to introduce innumerable other problems.

Case in point. What happens in this section of code if two threads pass the if conditional?

Code:

---

static T* Create()
{
    assert(m_Instances == 0); // Allow only one instance of the object
    if (m_Instances >= 1)
      return NULL;
    T* p = new T;
    m_Instances++;
    return p;
}

---

Lets say that thread A gets past the if conditional but then gets task switched. Now thread B comes in and also gets passed the if and then gets task switched. Now thread A picks up at the new and returns and then gets task switched. Thread B will resume at the new past the if conditonal and as such will create another instance of the object. Thus we have just created two instances of an object in code that was supposed to guarantee we only create one instance. As such this code does not guarantee one instance of an object in a multi-threaded environment.

Though I agree with the general sentiment of "if it should be a singleton allowing more than one instance is crazy", criticizing the code because of a threading issue that can't be solved within standard C++ is also crazy.

Soma

Quote:

---

Though I agree with the general sentiment of "if it should be a singleton allowing more than one instance is crazy", criticizing the code because of a threading issue that can't be solved within standard C++ is also crazy.

---

But it can be solved. The only way to ensure 1 instance via C++ is to have a CreateInstance() and GetInstance(). The CreateInstance() is called during some application startup code so that the instantiation is not determined by who calls GetInstance() first and thus is not prone to out of order instantiation errors or early instantiation. Lazy instantiation is very prone to these types of errors. GetInstance() in this case would only return the instance that was created.

And I'm not criticizing the code. But if the comment in the code reads this will create only 1 instance when there are clear instances when it will not, to me that is a bug waiting to happen. This can be a serious problem in a multi-threaded environment and could mean the difference between a device listener having the right object to listen to. If you created two device objects by mistake then theoretically half of it's listeners could attach to the wrong object thus causing them to miss very important calls. Granted I work in an embedded environment where this can bring the system crashing down hard but it could be an issue in other types of code as well.

Quote:

---

Originally Posted by Bubba

But it can be solved. The only way to ensure 1 instance via C++ is to have a CreateInstance() and GetInstance(). The CreateInstance() is called during some application startup code so that the instantiation is not determined by who calls GetInstance() first and thus is not prone to out of order instantiation errors or early instantiation. Lazy instantiation is very prone to these types of errors. GetInstance() in this case would only return the instance that was created.

---

That works, the cleaner solution is to use pthread_once or some similar thing in your MT lib.

So the singleton-MT problems are solved. But the trouble goes on if you ever start trying to refactor your singleton class into an .dll/.so or access your singleton from a .dll/.so. What do the static vars do? Have they still a single instance or do they get doubled/tripled/n'led? Imo there is no portable solution, so get rid of singletons in C++, just create one object, pass it's reference through your program like anything else, add a line of documentation and save you a lot of headache.

Quote:

---

Originally Posted by pheres

But the trouble goes on if you ever start trying to refactor your singleton class into an .dll/.so or access your singleton from a .dll/.so. What do the static vars do? Have they still a single instance or do they get doubled/tripled/n'led? Imo there is no portable solution, so get rid of singletons in C++, just create one object, pass it's reference through your program like anything else, add a line of documentation and save you a lot of headache.

---

How is creating one object somewhere at the beginning and passing it around different than initializing a singleton somewhere at the beginning of the program? Other than that the singleton doesn't, well, have to be passed around everywhere.

If some leaf or near-leaf class uses the singleton, it's easy to decouple if you later decide it doesn't need the singleton anymore. If you're passing around a reference everywhere, that refactoring could have a ripple effect.

Like anything, Singletons aren't a perfect solution to every problem and can be abused, but I largely prefer them than passing some master object (that's only protected from multiple instances by documentation) through myriad classes that are merely acting as "middle men".

Quote:

---

How is creating one object somewhere at the beginning and passing it around different than initializing a singleton somewhere at the beginning of the program? Other than that the singleton doesn't, well, have to be passed around everywhere.

---

Hiding dependencies is a big difference and disadvantage (except perhaps in rare cases).

Quote:

---

If some leaf or near-leaf class uses the singleton, it's easy to decouple if you later decide it doesn't need the singleton anymore. If you're passing around a reference everywhere, that refactoring could have a ripple effect.

---

Perhaps you may decide that you don't want logging (but then there should be better ways of disabling it rather than rewriting code) but how would you, for example, imagine a CWorld instance becoming unnecessary?

A singleton / global is probably a quick way to hack on new features if the interfaces aren't flexible enough for the new requirements. Isn't it nice that you don't have to change a bunch of signatures and calls to make more information available to things? The price is degradation of code which probably makes each new change more difficult.

Quote:

---

Like anything, Singletons aren't a perfect solution to every problem and can be abused, but I largely prefer them than passing some master object (that's only protected from multiple instances by documentation) through myriad classes that are merely acting as "middle men".

---

A master object is probably another code smell (God object) and there might still be too high coupling. I guess many of the other classes only need to know about some part of the master object which therefore should be broken up, so each part of the code can reference exactly what it needs and not more.

Quote:

---

Originally Posted by medievalelks

How is creating one object somewhere at the beginning and passing it around different than initializing a singleton somewhere at the beginning of the program? Other than that the singleton doesn't, well, have to be passed around everywhere.

---

various problems (regarding multi threading/initialization or access over shared library boundaries) are described inside this thread. In a bigger project with lot of programmers and a bunch of interdependent modules these problems can strike you if you don't expect it. Then refactoring from the singleton pattern to "reference passing" will be your smallest problem.

Quote:

---

Originally Posted by pheres

various problems (regarding multi threading/initialization or access over shared library boundaries) are described inside this thread.

---

A single instance of a non-singleton still has to have its creation managed somewhere in the program, and is subject to the same synchronization issues in a MT environment, no?

Quote:

---

In a bigger project with lot of programmers and a bunch of interdependent modules these problems can strike you if you don't expect it.

---

Cannot the same problem occur (more easily perhaps) with a single object passed by reference? What's to stop a programmer from constructing one where he thinks he needs it?

Isn't a key to MT to minimize the amount of shared data between the threads? As such wouldn't anything of which only one instance can exist be rather unsuitable to be used across threads?

Quote:

---

Cannot the same problem occur (more easily perhaps) with a single object passed by reference? What's to stop a programmer from constructing one where he thinks he needs it?

---

At least the dependencies are more visible. As to constructing more than one (CWorld), you'd probably notice and you can limit construction...

And of course, if there is and should only ever ONE, there are two obvious solutions to avoid it being created in multiple threads:
1. Create it before threads are started, in the main thread.
2. Create it on the stack in the main function.
Both or either can be used.

Assuming that the class object isn't large, this should work fine, and avoids all sorts of problems.

I agree that sharing should be avoided in a MT environment, as all shared data that is ever updated by any thread will be subject to locks to ensure that no other thread is getting inconsistent data.

--
Mats

Quote:

---

Originally Posted by Bubba

Case in point. What happens in this section of code if two threads pass the if conditional?

---

That is multi-threaded issues, and these issues affects everything you make, so it's hardly fair to say that it's a design flaw for this specific piece of code. It wasn't made to be thread safe. Can it be done? Sure. But at a cost. If you don't need it to be thread-safe, or if you want the caller to make it thread-safe, then why implement such a protection at all when all it does is add overhead?

Quote:

---

Originally Posted by medievalelks

A single instance of a non-singleton still has to have its creation managed somewhere in the program, and is subject to the same synchronization issues in a MT environment, no?

---

Of course all data access from different threads
must be serialized properly to avoid races. In many cases the designer can hide the thread safety code from the client. And if still something goes wrong: nearly every programmer will know how to help you securing your data access (much fewer know how to compute data in parallel using techniques which scale, but that doesn't count here)

But the issue with singleton instantiation is worse:

Code:

---

class C :public Singleton{};
...
void f1() {
C::instance(); }
...
void f2() {
C::instance(); }

---

How many objects do you have if this is a single threaded program? One.
How many objects does your mate who re-uses your object produce in his multithreaded multi-module program? You as the class designer can't know. If he doesn't know about the singleton problems he probably thinks one, because the code worked inside your program for years.
But some access to his singleton objects doesn't seem to go right although he secured every data access...
Now post this vague problem to a forum and better prepare some cans of coffee for the next few nights of code review...

Quote:

---

Originally Posted by medievalelks

Cannot the same problem occur (more easily perhaps) with a single object passed by reference? What's to stop a programmer from constructing one where he thinks he needs it?

---

Unfortunately nothing.
The singleton problems are not about programmers doing something wrong. Rather it is about things which looks right but break during the costumer demo.

Quote:

---

Originally Posted by pheres

Code:

---

class C :public Singleton{};
...
void f1() {
C::instance(); }
...
void f2() {
C::instance(); }

---

How many objects do you have if this is a single threaded program? One.
How many objects does your mate who re-uses your object produce in his multithreaded multi-module program? You as the class designer can't know.

---

How is any of that mitigated with a class that you intend, but don't enforce, to have only one instance?

For one thing this lazy instantiation is a bit of a problem. If you create a single instance normally or through Elysia's factory before other threads start, you won't have this problem.

Using the singleton in multiple threads might be quite problematic though.

Quote:

---

Originally Posted by medievalelks

How is any of that mitigated with a class that you intend, but don't enforce, to have only one instance?

---

You can't enforce singularity (if you consider re-usability - At least I don't know a method which is guaranteed to work under all circumstances. And if you search the net you will see I'm not alone). So what is left?
-Method a (singleton) which promises things it can't hold
-Method b - the standard method everybody knows to use

What of both would you put into your companies code base?

I think that it also tends to be more difficult to design unit tests involving singletons.

Seems like thread-safety could be enforced by a simple spinlock during object construction.

Code:

---

void SpinLock(int &lock);
void SpinUnlock(int &lock);

class TheSingleton
{
    static int mSpinLock;
    static bool mInstantiated;

public:
    static void DoInstantiate();

    static void Instantiate()
    {
        SpinLock(mSpinLock);
        if(!mInstantiated)
        {
            DoInstantiate();
            mInstantiated = true;
        }
        SpinUnlock(mSpinLock);
};

int TheSingleton::mSpinLock = 0;
bool TheSingleton::mInstantiated = false;

---

Sure, if two threads try to instantiate at the same time, one of them will spin for a while -- but it will only happen once, and ONLY if two threads try to instantiate at the same time.

There are no blocking waits, no OS dependencies.

The multithreading problem is easy to solve using pthread_once or similar. But what will happen if someone calls Instantiate() from a shared lib/dll/whatever_your_OS supports? This imo depends on the behavior of the OS.

Quote:

---

Originally Posted by pheres

The multithreading problem is easy to solve using pthread_once or similar. But what will happen if someone calls Instantiate() from a shared lib/dll/whatever_your_OS supports? This imo depends on the behavior of the OS.

---

The same thing that will happen if they invoke the constructor on your non-Singleton solution.

No, because they have no static flags which marks instantiated or not. These flags will either reside on a single one or more memory locations, depending on your OS. Try it out. Linux and WinXp should be enough to find a difference.