Thread: Compiler complaining about my singleton class

The compiler is complaining about my singleton class I am creating. It's pretty simple.

Here is the .h file:

Code:

class GAME_MANAGER
{
	private:

		//Single instance of the GAME_MANAGER class
		static GAME_MANAGER *gameManagerInstance;

		//Constructors and assignment operator - private
		GAME_MANAGER ( ) { };
		GAME_MANAGER ( const GAME_MANAGER &copy ) { };
		void operator = ( const GAME_MANAGER &rhs ) { };

	public:

		static GAME_MANAGER *GetInstance ( );

};

Here is the cpp file:

Code:

#include "game_manager.h"

GAME_MANAGER *GAME_MANAGER::gameManagerInstance = NULL;

static GAME_MANAGER::GAME_MANAGER *GetInstance ( )
{
	if ( !gameManagerInstance )
	{
		gameManagerInstance = new GAME_MANAGER();
	}

	return gameManagerInstance;
}

As far as I know this is the way singletons are supposed to be done in C++, but it is really complaining at me a lot.

The errors are as follows:

game_manager.cpp||In function `GAME_MANAGER* GetInstance()':|
game_manager.cpp|7|error: `gameManagerInstance' was not declared in this scope|
inc\game_manager.h|25|error: `GAME_MANAGER::GAME_MANAGER()' is private|
game_manager.cpp|9|error: within this context|
game_manager.cpp|12|error: `gameManagerInstance' was not declared in this scope|
||=== Build finished: 4 errors, 0 warnings ===|

Any ideas what is going wrong?

Oh...I am using Code::Blocks, and the compiler appears to be GCC 3.4.5.

Thanks

This:

Code:

static GAME_MANAGER::GAME_MANAGER *GetInstance ( )

should be:

Code:

GAME_MANAGER *GAME_MANAGER::GetInstance ( )

oops

thanks for catching that

Also, this:

Code:

void operator = ( const GAME_MANAGER &rhs ) { };

should be this:

Code:

GAME_MANAGER& operator = ( const GAME_MANAGER &rhs ) { };

And why all capital letters for the class name? That ugly naming convention is usually used for macros.

Originally Posted by cpjust

Also, this:

Code:

void operator = ( const GAME_MANAGER &rhs ) { };

should be this:

Code:

GAME_MANAGER& operator = ( const GAME_MANAGER &rhs ) { };

In this case it makes no difference since either way the copy assignment operator is disabled.

Originally Posted by R.Stiltskin

Does it make any sense to have either a copy constructor or an overloaded asssignment operator in a singleton class?

Probably not, since copying implies that you have more than one object.

Incidentally, the copy constructor and copy assignment operator do not have to be defined in order to be disabled.

Indeed, you should remove the implementations of copying and assignment, so that if you accidentally try to copy where they are even accessible you still get linker errors.

And why all capital letters for the class name? That ugly naming convention is usually used for macros.

Well, when I code in C++ my normal IDE is either Notepad++ or Code::Blocks. Consequently, class names are not highlighted any special color when I declare objects (Visual Studio, on the other hand, highlights the name of the class when you declare an instance of it...but I mostly only use Visual Studio for C#).

Consequently, to distinguish between variable names and class names in my C++ code I use all caps quite often for my class names. It makes the class name easier to distinguish from the rest of your code in an IDE that doesn't highlight them for you.

Then how do you distinguish class names from macros?

I just start class names with a capital letter and variables with a small letter...

Well, my macros are usually easily distinguishable by what they are.

For example:

AGENT_COUNT_DEFAULT

is obviously a macro (defined to 100 if you were wondering )

whereas:

GAME_MANAGER

is obviously a class.

I sometimes start out variables with a capital letter depending on what the variable is, but most of my variables start out with lowercase letters like you do.

I like to start the classes with a letter identifying the type, eg C. CGameManager. Then I know it's a class no matter what I name the instance of it.
But... you do know that you shouldn't really use macros in C++, yes? A constant integer/float/whatever will do the trick instead.