Singleton template problem

Hi all,

I've implemented my own singleton pattern using templates for my classes to derive from. Here is a very cut down version of my code:

Code:

// THIS FILE IS SINGLETON.H

template <typename T>
class Singleton
{
public:
    static T* CreateSingleton( )
    {
        if (m_Instance == NULL)
            m_Instance = new T ; // ERROR HERE
        return m_Instance ;
    }
private:
    static T* m_Instance ;
};

// initialize instance to NULL
template<typename T>
T* Singleton<T>::m_Instance = NULL ;

the above is the singleton file, as I said it is only a snippet of the code.

This is how you derive from the singleton class

Code:

#include "Singleton.h"

class MyClass : public Singleton<MyClass>
{
};

And this is how you use it

Code:

#include "MyClass.h"

int main( )
{
    MyClass* mc = MyClass::CreateSingleton( ) ;
    return 0 ;
}

Now I have seen code like this all over the internet and people seem to be using it fine. However when I try to compile it in g++ I get an error in Singleton.h on the line that says

Code:

m_Instance = new T ;

The error is "undefined type MyClass". Now I completely understand this error because the singleton class is trying to "new" an instance of type T. However because I havent included "MyClass.h" at the top of "Singleton.h" it doesnt know about the class MyClass. For obvious reasons I dont want to add that to Singleton.h

Now im usually fine with this kind of stuff so im putting it down to im having a blonde day. But for the life of me I cannot find away around this problem, and whats more confusing is that other people are using this technique with no issues.

Thanks for any help

You didn't post enough code. The code you posted compiles fine with G++, as I expected.

The only reason this doesn't compile for me is because NULL is a macro and you need to include some header for it.

Incidentally, the macro defines NULL as 0, so you might just use 0 instead and not include anything.

Originally Posted by anon

The only reason this doesn't compile for me is because NULL is a macro and you need to include some header for it.

Incidentally, the macro defines NULL as 0, so you might just use 0 instead and not include anything.

It's generally better style in C++ to use "0" instead of "NULL" anyway.

IMO its better to use NULL, particularly for pointers, since that indicates tot eh reader that the field is more or less 'unused', rather than passed a value of zero instead of some other possibly valid value. Generally code written to accept pointers changes it behavior when passed NULL, but that migth not be obvious if you use 0, since a lot of code has active states that use 0, such as passing the index into an array.

I believe NULL is defined in windows.h along with a lot of other useful macros, if you are writign for linux you can just add -

Code:

#define NULL 0

windows.h might define NULL, but the proper place for it in C++ is <cstddef>.

Originally Posted by abachler

IMO its better to use NULL, particularly for pointers, since that indicates tot eh reader that the field is more or less 'unused', rather than passed a value of zero instead of some other possibly valid value. Generally code written to accept pointers changes it behavior when passed NULL, but that migth not be obvious if you use 0, since a lot of code has active states that use 0, such as passing the index into an array.

On the other hand, this can be misleading since a mistake could cause one to pass NULL where an integer, not a pointer, is expected, and the compiler would happily accept it since NULL is indeed 0. We'll just have to wait for nullptr to save us.

Originally Posted by abachler

I believe NULL is defined in windows.h along with a lot of other useful macros

It might be, but the correct C++ standard header is <cstddef>.

windows.h might include cstddef.h, I haven't read through it recently.

From teh compilers standpoint it doesnt matter, but contextually it makes the code easier to understand when I see NULL, I know immediately, ok that variable isnt important in this call. Whereas if I see 0, I have to keep it in mind. Then again, I have worked on code where a pointer to memloc 0 was a valid pointer, so if i saw a function call that refernced address 0 it was actually intended to modify that memory, whereas a cvalue of NULL lets me know that it does not intend to use it.

On the other hand, GCC defines NULL to be __null, which is basically the integer constant 0, but will emit a warning if it is actually used as an integer. So NULL does have its advantages.

Which is why I agree with abachler. Use NULL for pointers, not 0.
I think it is pretty safe to guess that almost every header actually includes cstddef, or defines NULL itself. However, it might not be as good as thing to rely on.

Only stdlib.h/cstdlib and stdio.h/cstdio are guaranteed to define NULL, besides stddef.h/cstddef of course. No other header guarantees availability of the macro.

Thanks all for the reply,

I had a bit of a jiggle around with the code and I managed to get the exact same code spitting out another error which was "undefined reference to vtable".

I understand what the vtable is but I have never come across this error with the microsoft compiler only g++.

The gcc faq mentions the following:

The ISO C++ Standard specifies that all virtual methods of a class that are not pure-virtual must be defined, but does not require any diagnostic for violations of this rule [class.virtual]/8. Based on this assumption, GCC will only emit the implicitly defined constructors, the assignment operator, the destructor and the virtual table of a class in the translation unit that defines its first such non-inline method.

Therefore, if you fail to define this particular method, the linker may complain about the lack of definitions for apparently unrelated symbols. Unfortunately, in order to improve this error message, it might be necessary to change the linker, and this can't always be done.

The solution is to ensure that all virtual methods that are not pure are defined. Note that a destructor must be defined even if it is declared pure-virtual [class.dtor]/7.

When I define the virtual methods in the cpp file I get an "undefined reference to vtable". However when I define the methods with the declaration in the header file, everything compiles.

I have never used g++ so its probably me just missing something but surely you're not expected to define all the virtual members in the header files?

The IDE im using is codeblocks.

Thanks again

I can't see what all the virtual things has to do with Singleton that consists entirely of static things. May-be you could post your new code?

Unfortunately I dont have access to the code at the moment (I can get it again tomorrow).

The issue seems to be that the classes deriving from the Singleton pattern all have a virtual destructor (because they are deriving publicly from the singleton class).

If you look at the singleton code I posted above there is an extra function declared like so:

Code:

public :
 virtual ~Singleton( ) { }

this is obviously the destructor and as you can tell it is inlined.

Now the code compiles fine if I also define the derived classes destructors inline like the singleton class. However if I define them in the cpp file and not the header file then I get this "unreference vtable" error.

Hopefully that makes sense.

Thanks again for your help

Make the destructor non-virtual and protected, not virtual and public. There's absolutely no reason for it to be public or virtual.