CODEWITHSUNDEEP
c++singleton
Yogi
Yogi

Reputation: 1047

Global Object in singleton

In the Singleton pattern why is that we have to use a static object and not a global object?

I tried the following

class Singleton;
Singleton*  instance1 = NULL;

class Singleton
{
private :
       Singleton()
       {
       }
       //static Singleton* instance1;

public:
        static Singleton* getinstance()

 if(instance1== NULL)
       {
       instance1 = new Singleton();
       }

 return instance1;


 void Dispaly()
       {
       }
 ~Singleton()
       {
       }
};

When I compile this code I get the error "multiple definition of instance1"

Can anyone please give a plausible reason for this?

Upvotes: 0

Views: 2455

Answers (8)

Sarfaraz Nawaz
Sarfaraz Nawaz

Reputation: 361252

If you make it a global object, then everyone has access to it, and anyone can use it without calling getInstance(). If so, then what is the purpose of getInstance() then? First time, you will call it to create the instance, then you wouldn't be required to call getInstance(), since after the first call, you can directly use instance.

A private static instance gives you more control how it can be accessed: only through by calling getInstance().

Now why you get multiple definition of instance error when compiling your code? Its because you've defined the global object in the header file itself, which is included in multiple .cpp files. This causes multiple definitions of the object, one definition in each translation unit (.obj file).

What you actually should be doing is this:

//Singleton.h
class Singleton
{
private :
        Singleton();
        Singleton(const Singleton &);
       ~Singleton();

        static Singleton* instance; //declaration of static member!

public:
        static Singleton* getInstance();
       //..
};

And then define the static member in the .cpp file as:

//Singleton.cpp
 Singleton *Singleton::instance = 0; //definition should go in .cpp file!

 Singleton::Singleton() {}

 Singleton* Singleton::getInstance()
 {
     if ( instance  == 0 ) instance  = new Singleton();
     return instance;
 }
 //...

Upvotes: 4

Mike Seymour
Mike Seymour

Reputation: 254431

Assuming this is part of a header file, then you'll get one definition of instance1 for each translation unit that includes it, and so a multiple definition error if more than one includes it. You'll get this problem whether it's a global or a static member.

To fix the error, you need a declaration in the header file (extern Singleton * instance1; if you want it to be global for some reason, or a private static member declaration if you want to prevent arbitrary access to it), and then a definition in a source file.

Alternatively, you could make the instance a static local object in getinstance(), which will fix the memory leak and, in C++11 at least, the thread safety problem:

static Singleton & getinstance()
{
    static Singleton instance;
    return instance;
}

Or you could not use a singleton at all - it's almost certainly not a good idea.

Upvotes: 0

ks1322
ks1322

Reputation: 35708

In the Singleton pattern why is that we have to use a static object and not a global object?

Static object is private member of your class and nobody has access to it except Singleton class. This is basic encapsulation principle in C++ and other OOP languages.

Upvotes: 0

Pete
Pete

Reputation: 4812

Put the definition of instance1 in a cpp file and an extern in the header.

It would be more typical to use a static class member though, as you have commented out. You just need to define it in a cpp file:

Singleton* Singleton::instance1 = 0;

Upvotes: 0

Luchian Grigore
Luchian Grigore

Reputation: 258548

That defeats the purpose of the singleton - accessing the object from a single place. You can access it everywhere without having to call getInstance().

As for the error, my guess is you included your header in multiple files.

Put

#pragma once

at the beginning of your header file, that should solve the error.

But again, this is a bad implementation of the singleton pattern.

EDIT:

Or use header guards:

#ifndef _SINGLETON_CLASS_INCLUDED
#define _SINGLETON_CLASS_INCLUDED

//// contents of your header

#endif

Upvotes: 0

Bo Persson
Bo Persson

Reputation: 92211

If you include this header in multiple files, you will get a copy of the pointer in each file.

That's probably what the linker is complaining about.

Upvotes: 3

Oliver Charlesworth
Oliver Charlesworth

Reputation: 272467

To answer your direct question:

The reason you get the error message is because you are defining a variable in a header file (well, that's my guess, anyway). So every time you #include that header file, the variable will get re-defined.

In summary, there's almost never a good reason for defining variables in header files.

[Side note: as others have explained, this is a very bad implementation of the singleton pattern.]

Upvotes: 4

duedl0r
duedl0r

Reputation: 9414

This is a very bad singleton implementation. You should use an implementation which is already here. There are a lot of them.. and your error will disappear.

For example, someone can set instance = NULL;. Doing that you can instantiate more than one instance.

Upvotes: 0

Related Questions