views:

641

answers:

6

Hi,

I'm currently trying to implement a factory as a singleton. I practically used the textbook example of the Singleton pattern. Here's the .h file:

namespace oxygen{

class ImpFactory{

public:
    static boost::shared_ptr<ImpFactory> GetInstance();

private:
    static boost::shared_ptr<ImpFactory> mInstance;
};

and here's the .cpp file:

#include "impfactory.h"

using namespace oxygen;
using namespace boost;

shared_ptr<ImpFactory> ImpFactory::GetInstance(){
    if (mInstance.get() == 0)
        mInstance = shared_ptr<ImpFactory>(new ImpFactory());
    return mInstance;
}

The code compiles, but I get a linker error:

../../lib/oxygen/liboxygen.so.3.2.4: undefined reference to `oxygen::ImpFactory::mInstance'

This currently has three students stumped. Any ideas?

+2  A: 

You must define the static instance, not just declare it. The definition creates the actual object you refer to.

In your cpp file, add the line:

boost::shared_ptr<ImpFactory> ImpFactory::mInstance;
Eli Bendersky
+2  A: 

You need a definition for your static member in a cpp file.

boost::shared_ptr<ImpFactory> ImpFactory::mInstance;
Nikola Smiljanić
+1  A: 

In your c++ add this:

boost::shared_ptr<ImpFactory> ImpFactory::mInstance;
VDVLeon
A: 

As an aside, note that quite a few seasoned developers think that singletons are bad. See What is so bad about singletons? for example.

MadKeithV
A: 

on another side note, maybe you should make the instance pointer a static member of the get function rather than the class, this doesn't make too much difference when using the new/pointer method you are. but if you were just creating a static instance (ie not using a pointer, and returning a reference to it from get get function) this makes a big difference because:

if its a static member of a class, its constructor is called whenever (because its a global) if its a static member of the get function, it isn't constructed untill it's called the first time, this alleviates some of the problems people have with singletons and them being glorified globals, the other good thing is, most linkers will omit the get function and hence the static instance entirely if it is never called, thus you don't have to worry about calling new so that it only uses memory if it's being used.

matt
A: 

Since you're using Boost, you might want to consider the Boost singleton classes. Check out:

#include <boost/serialization/singleton.hpp>

using namespace boost::serialisation;

struct MyClass : public singleton<MyClass>
{ string name_; int age_; };

int main( int argc, char* argv[] )
{
    MyClass::get_mutable_instance().name_ = "Robin";
    MyClass::get_mutable_instance().age_ = 21;
}

Which you use depends on what you're doing. While I'm a bit anti-singleton for the usual reasons, it makes sense to re-use where possible. A word of warning though: the Boost singleton appears to have moved about a bit in the libraries so this may vary depending on which version of Boost you're using.

Robin Welch
I think it's not so much "moved about" as "added when someone thought they wanted a singleton, then removed when they realised it wasn't such a good idea after all".
Mike Seymour