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)

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

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.

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.