template <typename T> class Singleton
{
static T * ms_Singleton;
public :
Singleton(void)
{
assert(!ms_Singleton);
int offset = (int)(T *)1 - (int)(Singleton <T> *)(T*)1;
ms_Singleton = (T*)((int)this + offset);
}
~Singleton()
{
asset(ms_Singleton);
ms_Singleton = nullptr;
}
static T & GetSingleton()
{
return (*ms_Singleton);
}
static T * GetSingletonPtr()
{
return (ms_Singleton);
}
};
template <typename T> T* Singleton<T>::ms_Singleton = 0;
class Manager : public Singleton <Manager>
{
public :
void Print()
{
std::cout << "Manager Instance called" << std::endl;
}
};
int main()
{
Manager *m = Manager::GetSingletonPtr();
m->Print();
}
Singleton Template questions - Code from Game Programming Gems
Hi,
I have some code from Game Programming Gems 1 and I have a few questions on it.
The Manager pointer in main actually on debugging gives me a NULL pointer - 0x0000 but it still seems to work fine. How is this possible ? Secondly how does this code snippet exactly work. What exactly is happening here.
int offset = (int)(T *)1 - (int)(Singleton <T> *)(T*)1;
ms_Singleton = (T*)((int)this + offset);
Thanks
Quote:The Manager pointer in main actually on debugging gives me a NULL pointer - 0x0000 but it still seems to work fine. How is this possible ? Secondly how does this code snippet exactly work. What exactly is happening here.
Make sure your not in release mode as that can cause symbols not to be updated but still show up in the debugger.
int offset = (int)(T *)1 - (int)(Singleton <T> *)(T*)1;ms_Singleton = (T*)((int)this + offset);
BLOWS UP THE UNIVERSE!!!!!!!!!!!!!!!!But more on that soon.
What its intended to result in is offset = num bytes from base class pointer to derived class pointer and ms_Singleton = this as a derived pointer.
Now to expand on my earlier comment. Seriously those two line have so much undefined behaviour i dont know where to start. The two (T*)1's in the first line produces undefined behaviour you could avoid this with a reinterpret_cast<T*> but that results in the incorrect result (compared to what was intended) static_cast<T*> has the right semantics but results in the same undefined behaviour as (T*)1. Then casting the pointer to an int results in more undefined behaviour, reinterperat_cast<int>() could be used to avoid that bit of udefined behaviour but then any operation on the result except for reinterperate_cast<T*>() results in more undefined behaviour (in this case the subtraction) and finally you run in to the same problems trying to add the result to (int)this, which should be reinterpret_cast<int>(this) to avoid undefined behaviour, and casting that back to a T*.
int offset = (int)(T *)1 - (int)(Singleton <T> *)(T*)1;ms_Singleton = (T*)((int)this + offset);
Maybe a silly question, but why couldn't they just do:
ms_Singleton = static_cast<T*>(this);
Is use of static_cast() undefined when casting to "type*" when that object of that "type" is not yet fully constructed? IIRC using "this" as a pointer (or reference - "*this") just to pass the pointer around or store it somewhere is fully reliable (the standard points it out explicitly somewhere).
Quote:Is use of static_cast() undefined when casting to "type*" when that object of that "type" is not yet fully constructed? IIRC using "this" as a pointer (or reference - "*this") just to pass the pointer around or store it somewhere is fully reliable (the standard points it out explicitly somewhere).
IIRC static_cast<>() was poorly supported by compiler vendors when the book was written (same goes for the other *_cast<>()'s).
That code doesn't work, since it never allocates any objects - you have to do something like else it'll crash when you try accessing any member variables of Manager, because (as you saw) m==NULL in your original code. The Print() method only works because it's not using 'this' at all.
int main(){ new Manager(); Manager *m = Manager::GetSingletonPtr(); m->Print(); delete Manager::GetSingletonPtr();}
Yeah. I've seen (unfortunately) many implementations of Singleton, and this one might be the worst. If the authors wanted to make a singleton that requires explicit instantiation (which they've done), then at a *minimum*, the two accessors (two? why? why!?!?!?) should ASSERT that ms_Singleton is not NULL.
But yeah, that constructor is just a pile of disasters waiting to happen.
But yeah, that constructor is just a pile of disasters waiting to happen.
Scott Bilas, the author of this particular Gem, has additional notes on his website regarding its implementation. He acknowledges the flaw in the constructor and recommends the static_cast instead.
Personally I prefer this version of the Singleton (in the rare event that I actually need one):
template<typename T>class Singleton{public: static T& Get() { // This bit is the magic. The standard guarenttees that static // local variables will be initialized the first time that the // function is called. This means that order of construction is // guarentteed and automagically occurs the first time Get() is // called for the type T, regardless of where it's called from // (eg. calling it from a global's or another singleton's // constructor is perfectly safe). static T instance; return instance; }private: Singleton() {}};class MyClass{public: void DoStuff() {}private: // You only need this bit if you want to ensure // the class is only used as a singleton friend class Singleton<MyClass>; MyClass() {}};void func(){ Singleton<MyClass>::Get().DoStuff();}
Agreed with joanusdmentia. Singletons are the anti-OO design pattern and should be used when NOTHING else works. I've yet to come across a situation where a singleton wasn't just lazy programming. Logger? No need to be a singleton, you've over designed it most likely, a few handy global functions (in a namespace of course) would be much better. Consider the following as an example:
Log::Get().Write(..., ...);
vs
Log(..., ...);
I think using the second one is a lot cleaner. The global function can access a class with static members if needed.
Normally exposing a class to EVERYTHING should be a huge red flag of a really bad design going on. Time to refactor some code.
Log::Get().Write(..., ...);
vs
Log(..., ...);
I think using the second one is a lot cleaner. The global function can access a class with static members if needed.
Normally exposing a class to EVERYTHING should be a huge red flag of a really bad design going on. Time to refactor some code.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement