开发者

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?


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;
 }
 //...


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.]


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.


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.


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


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;


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.


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.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜