Hey everyone! First off, lemme just say thanks for reading :). I'm trying to create a singleton class for a game project. I think it's almost there, I used the tutorial found at http://www.gameprogrammer.org/main.php?page=tutorials&no=73 but I'm having a little bit of trouble getting the last pieces to work. I'm using it to make an image manager, and the error output is the following: undefined reference to `singleton<imgmgr>::getInstance()' I tried all kinds of things and I must admit I'm clueless as to why this error occurs. Can someone maybe point out what's wrong with the code? I call the singleton by doing imgmgr::getInstance() as I read in the tutorial but somehow it fails to work. Anyway, I'd appreciate any help. I've included the code of the two classes below :) In imgmgr.h: #ifndef IMGMGR_H_ #define IMGMGR_H_ #include <list> #include "imgdata.h" #include "singleton.h" using namespace std; class imgmgr : public singleton<imgmgr> { public: void load(char* filename); protected: imgmgr() {}; ~imgmgr(); friend class singleton<imgmgr>; list<imgdata> pool; }; ------------------- In singleton.h: #ifndef SINGLETON_H_ #define SINGLETON_H_ template <class T> class singleton { public: static T *getInstance(); protected: singleton(); private: static T *instance; }; #endif ------------------- In singleton.cpp: template<class T> singleton<T>::singleton() { instance = 0; } template<class T> T * singleton<T>::getInstance() { if(instance == 0) { instance = new T(); } return instance; } -------------------

On the subject of singletons, do not use them. Promit has collected a list of further reading on the stupidity of singletons and the flaws they represent.

Your problem is with the template. The entire definition of a template must be available to the compiler at the point where the template is being instantiated. This means, effectively, that you cannot put your template class's method implementations in a source file. Define them in the header.

The rest of this post is advice, but is not directly related to your question.

Also, using directives (using namespace std) in headers are poor practice because you pollute the global namespace. Either restrict the scope of the using directive, or explicitly qualify the namespace; doing it globally subverts the entire point of namespaces.

Also, since you're obviously using C++, prefer std::string to char* for string data. You can use the std::string::c_str() method to get a const char* to pass to legacy APIs that do not support std::string.

Also, your attempt to suppress construction of imgmgr is unnecessary (you have enough self-control not to do it "accidentally") and dangerous. You need to make both the constructors and the copy constructor and assignment operators protected, but leave the destructor public (it cannot be destroyed if it cannot be created, after all, and there are some instances where you can cause leaks by suppressing the destructor in this fashion).

Finally, I don't have enough information to tell what this class is supposed to do (it's name is very poor), but I suspect that std::list is not necessarily the best container for the job. A vector or deque are usually better choices by default, unless you specifically need the benefits of a linked list (mainly, the ability to cheaply insert in the middle when you have appropriate iterators).

[Edited by - jpetrie on June 21, 2007 12:17:31 PM]

Thanks for responding and the tips!

As far as I could gather from the first link, it's better for me to just stick to my old extern global variables instead of using singletons? Though I thought extern was just as bad, since it's hard to tell when it's gonna be initialized and all.

I'll definitely use string instead of char, and I'll avoid the namespace issue. :)

So again, thanks!

Quote:
As far as I could gather from the first link, it's better for me to just stick to my old extern global variables instead of using singletons? Though I thought extern was just as bad, since it's hard to tell when it's gonna be initialized and all.

Ideally, you would remove both, although in general globals are often slightly less dangerous than a singleton. That's just superficial stuff though. The bigger issue to address is... what does this class do, and why do you think you need it to be accessible everywhere? There is a very good chance that whatever answer you have to that (latter) question is wrong, and that you could end up with a better design by just passing a reference to the appropriate instance where-ever you needed it (remember, you don't need it as often as you think you do!)

Well that might actually be true. I'm gonna be using this class as a resource holder for all my images. So every time I for example create a new shot, it tells the resource holder to give the reference to a SDL_Surface. I think I might be able to pass it as a reference, I think I might give that a try. Thanks for your replies. :)