C++ Trying singleton behavior, keep getting 'undefined reference to Class::instance()

x_terminat_or_3 · 05-12-2008, 04:04 PM

Hi there,

I'm trying to get my head around C++ programming and would appreciate some help here.

First, I'm trying to abstract database access by making a database class.

I want singleton behavior, but it keeps telling me

'undefined reference to `Database::getInstance()' ' even though I made it public static.

Where did I go wrong?

Code:

main.cpp

#include "src/Database.h"

int main() {

	const Database* db=Database::getInstance();
}

Code:

Database.h

class Database {

	protected: 
		static Database* _instance;

	public:
		static Database* getInstance();

	protected:

		Database();
};

Code:

Database.cpp
#include "/usr/include/mysql/mysql.h"
#include <stdio.h>
#include "Database.h"


Database* Database::getInstance() {

	Database* out=Database::_instance;
		
	if(!(out)) {

		out=new Database();
	}
			
	return out;
}


Database::Database() {

	int blabla=0;
}

Any help is appreciated. Thanks

R00ts · 05-12-2008, 05:06 PM

You need to initialize your static member in your .cpp file. Do this near the top of the file:

Code:

Database::_instance = NULL;

x_terminat_or_3 · 05-12-2008, 05:31 PM

I've changed it to this:

Code:

Database.cpp
#include "/usr/include/mysql/mysql.h"
#include <stdio.h>
#include "Database.h"

Database* Database::_instance = NULL;

Database* Database::getInstance() {

	Database* out=Database::_instance;
		
	if(!(out)) {

		out=new Database();
	}
			
	return out;
}


Database::Database() {

	int blabla=0;
}

But it still throws the same error:

Quote:

main.cpp:5: undefined reference to `Database::getInstance()'

BrianK · 05-12-2008, 05:40 PM

how are you compiling it? Are you linking Database.o?

to save back & forth time, you should be doing something like:

g++ -c Database.cpp <-- this will make Database.o
g++ -o main Database.o main.cpp <-- this will make an executable called main

x_terminat_or_3 · 05-12-2008, 05:48 PM

That did the trick. Thanks

I was using kdevelop with the automake tools, obviously did something wrong there then.

Thanks for your help.

tuxdev · 05-12-2008, 10:41 PM

That's not a singleton. A standard singleton pattern uses function-local statics like:

Code:

class Database {

	public:
		static Database &get();

	protected:

		Database();
};

and

Code:

Database &Database::get() {
	static Database db;	
	return db;
}

Now, this isn't the only good pattern for this, but it's the safest one that I can use without going through somersaults. For example, if you need to have multiple implementations of a Database interface, you'll need to handle the singleton in a much more complicated fashion. I using the smaller method name to acquire the instance because that part really isn't important, but what you do with it. I give more space for the important stuff and less space for stuff I don't really care about.

dmail · 05-13-2008, 06:29 AM

Quote:

Originally Posted by tuxdev

That's not a singleton.

I will come back to this.

Quote:

A standard singleton pattern uses function-local statics like:

Who says? It is a singleton if it allows only one instance which is made global, which is the definition of a singleton.

Now can you see any problems with your class? It is possible to create more than one instance see the below code snippet.

Code:

int main() {

	const Database* db=Database::getInstance();
        Database foo(*db);
        assert(&foo == db);
}

For a generic Singleton template see http://www.liamdevine.co.uk/code/singleton.php

tuxdev · 05-13-2008, 09:56 AM

Quote:

Who says? It is a singleton if it allows only one instance which is made global, which is the definition of a singleton.

Scott Meyers, he's the guy who thought that version up and made it famous. I never said it was the only way, either.

Quote:

Now can you see any problems with your class? It is possible to create more than one instance see the below code snippet.

Sorry, I neglected to put the copy constructor in the in the right place. You didn't have to blow the mistake out of proportion, though.

Code:

class Database {

	public:
		static Database &get();

	protected:

		Database();
		Database(const Database &);
};

dmail · 05-13-2008, 10:16 AM

Quote:

Sorry, I neglected to put the copy constructor in the in the right place. You didn't have to blow the mistake out of proportion, though.

Sorry you have misinterpreted that it was intended for the OP, I realise your code was just an example.

x_terminat_or_3 · 05-13-2008, 12:56 PM

Thanks all, to wrap it up, could you guys tell me what exactly the difference is between Database* and Database& both are references no?

tuxdev · 05-13-2008, 02:01 PM

Quote:

Thanks all, to wrap it up, could you guys tell me what exactly the difference is between Database* and Database& both are references no?

Database * is a pointer, which you might end up deleting somehow. It's much harder to delete a reference like Database &. Also, you are communicating to the programmer using this singleton that the instance returned is never null, since references can never be null (actually, they can, but that's another long obtuse discussion about C++ and compiler implementations). Some singletons need construction parameters, so in that case the singleton acquisition would return a pointer since it should return null if the proper initialization hasn't happened yet.

Quote:

Sorry you have misinterpreted that it was intended for the OP, I realise your code was just an example.

Okay, we're cool then ;-). The original code actually has a more fundamental problem than copy construction. getInstance never changes _instance, so each call to it makes a new object.

@xterm: It's a generally bad idea to use names that start with _, it's possible that those are reserved by the compiler (there's complicated rules as to what exactly is and is not reserved and it's easier to avoid the situation)