CODEWITHSUNDEEP
c++design-patternssingleton
user1312703
user1312703

Reputation:

Why this singleton implementation doesn't compile?

I need a singleton implementation without using dynamic memory allocation. I tried to implement it like this:

// Singleton.hpp

class Singleton
{
    public:
    static Singleton& GetInstance();

    private:
    Singleton();
    static Singleton& Instance;
};

// Singlton.cpp

Singleton& Singleton::GetInstance()
{
    return Singleton::Instance;
}

Singleton::Singleton()
{
}

As I said this doesn't compiles. I read many articles, I tried to initialize static Singleton& Instance in different ways, but all I get is a new compilation errors. Why this doesn't work? And how to implement a singleton pattern without using dynamic memory allocation?

Upvotes: 0

Views: 342

Answers (4)

Rabbiya Shahid
Rabbiya Shahid

Reputation: 422

This is what I did for my singleton class:

I added a data member static int Count ; into my class.

So now I have :

a declaration static Singleton::Count = 0;

and the constructor:

 Singleton()
{
 Count++;
}

My getinstance is:

static Singleton GetInstance() 
    {
       Singleton obj;

    if ( Count == 1)

     return obj;

    }

Upvotes: 0

Henrique Barcelos
Henrique Barcelos

Reputation: 7900

This way:

class Singleton
{
    public:
        static Singleton& getInstance();
        ~Singleton();

    private:
        Singleton();
        Singleton(Singleton&); // don't implement
        void operator=(const Singleton&); // don't implement
};


#include "singleton.hpp"

Singleton& Singleton::getInstance()
{
    static Singleton instance;
    return instance;
}

Singleton::Singleton()
{
}

Singleton::~Singleton()
{
}

Edit:

Following your "OPP-way" of doing it, you just have to initialize your instance variable:

class Singleton
{
    public:
        static Singleton& getInstance();
        ~Singleton();

    private:
        Singleton();
        Singleton(Singleton&); // don't implement
        void operator=(const Singleton&); // don't implement
        static Singleton instance;
};

Singleton Singleton::instance;

Singleton& Singleton::getInstance()
{
    return instance;
}

Singleton::Singleton()
{
    std::cout << "ctor" << std::endl; 
}

Singleton::~Singleton()
{
}

Test with this:

int main() {
    Singleton& s = Singleton::getInstance().getInstance();
    s.getInstance();
}

The constructor will be called just once... But this way you loose the lazy initialization.

Upvotes: 0

Etherealone
Etherealone

Reputation: 3558

Your definition of Instance should not be reference like Singleton& Instance; in the header. It needs to be Singleton Instance; as it is not a reference but an object.

You have declared Singleton::Instance in the header but not defined it in the source. You need to put Singleton Singleton::Instance; in the source file. Non-const static objects need to be defined in the source file.

You need to declare getInstance method as static so that you won't need an actual object to call it. When it is static, you may call it using Singleton::getInstance();.

Upvotes: 0

Mark Ransom
Mark Ransom

Reputation: 308462

You need to declare the GetInstance method static as well, otherwise you can only call it on an existing object - and you can't create an existing object because the constructor is private.

P.S. instead of creating the instance as a static member of the class, you can make it a static local variable inside GetInstance.

P.P.S. I just noticed that your Instance variable is a reference that you didn't initialize - that won't work either. Here's my version:

Singleton & GetInstance()
{
    static Singleton Instance;
    return Instance;
}

Upvotes: 2

Related Questions