4

I'm trying to implement this singleton class. But I encountered this error:

'Singleton::~Singleton': cannot access private member declared in class 'Singleton' This is flagged in the header file, the last line which contains the closing brace.

Can somebody help me explain what is causing this problem? Below is my source code.

Singleton.h:


class Singleton
{
public:
    static Singleton* Instance()
    {
        if( !pInstance )
        {
            if( destroyed )
            {
                // throw exception
            }
            else
            {
                Create();
            }

        }
        return pInstance;
    }
private:
    static void Create()
    {
        static Singleton myInstance;
        pInstance = &myInstance;
    }
    Singleton() {}
    Singleton( const Singleton& );
    Singleton& operator=( const Singleton& );
    ~Singleton() 
    {
        pInstance = 0;
        detroyed = false;
    }

    static Singleton* pInstance;
    static bool destroyed;
};

Singleton.cpp:


Singleton* Singleton::pInstance = 0;
bool Singleton::destroyed = false;

Inside my main function:


Singleton* s = Singleton::Instance();

If I make the destructor as public, then the problem disappears. But a book (Modern C++ Design) says it should be private to prevent users from deleting the instance. I actually need to put some code for cleanup for pInstance and destroyed inside the destructor.

By the way, I'm using Visual C++ 6.0 to compile.

jasonline
  • 8,646
  • 19
  • 59
  • 80
  • it compiles flawlessley on g++ 4.2.4 – sud03r Jan 25 '10 at 07:49
  • I think it should work and can u give code flashing an error ? – Ashish Jan 25 '10 at 07:52
  • 1
    @Mac: Code flashing? I'm not sure I understand what this means. – jasonline Jan 25 '10 at 07:54
  • Neeraj: It does? So I guess this only occurs in Visual C++?! – jasonline Jan 25 '10 at 07:56
  • @jasonline - I think Mac means to show the actual code that produces the error (along with an indication for which line the error is flagged on). – Michael Burr Jan 25 '10 at 07:57
  • Where is the destructor supposed to be used anyway? I cant see anything in the posted code that would call it. By the way, shouldnt that singleton pattern have a Destroy() method? – Mizipzor Jan 25 '10 at 07:58
  • i compiled only the part of code you posted, as Samuel in his answer says the other parts may have some problem. But if it is from somewhere else, the error message is definitely not helpful – sud03r Jan 25 '10 at 08:00
  • I was thinking it may be due to creation of the static object? After the program ends, it needs to call the destructor right. But since the destructor is private... – jasonline Jan 25 '10 at 08:07
  • Why did I get a down vote? I mean it's really occurring in VC6. :( – jasonline Jan 25 '10 at 08:14
  • 1
    If that really is some weird VC6 bug and you don't need any behaviour in the destructor you could use `static Singleton* instance = new Singleton;` - or just don't use the singleton pattern and use a free function that returns the instance in question. – Georg Fritzsche Jan 25 '10 at 08:25

5 Answers5

2

The code you posted looks doesn't have any problem, so the problem must be in some other part of your source code.

The error message will be preceded by the filename and line number where the problem is occurring. Please look at the line and you'll see a bit of code that is either trying to call delete on a singleton pointer or is trying to construct an instance of singleton.

The error message will look something like this (the file and line number are just an example):

c:\path\to\file.cpp(41) : error C2248: 'Singleton::~Singleton': cannot access private member declared in class 'Singleton'

So in that case, you would want to see what is happening at line 41 in file.cpp.

R Samuel Klatchko
  • 74,869
  • 16
  • 134
  • 187
  • It's actually pointing at the header file, at the end of the file (the closing brace). And those are the only source codes in my workspace right now. – jasonline Jan 25 '10 at 08:01
  • Does this happen when compiling singleton.cpp or some other source file? What is the code right before `#include "singleton.h"`? – R Samuel Klatchko Jan 25 '10 at 08:14
  • @R Samuel Klatchko: Compiling singleton.cpp. Just another include file for stdafx.h. Someone was already able to reproduce it in one of the answers posted. – jasonline Jan 25 '10 at 08:19
2

You should probably have let us know that the version of Visual C++ you're working with is VC6. I can repro the error with that.

At this point, I have no suggestion other than to move up to a newer version of MSVC if possible (VC 2008 is available at no cost in the Express edition).

Just a couple other data points - VC2003 and later have no problem with the Singleton destructor being private as in your sample.

Michael Burr
  • 333,147
  • 50
  • 533
  • 760
  • @Michael: Sorry for missing that info. Thank you for your information. – jasonline Jan 25 '10 at 08:12
  • VC6 is pretty dang old nowadays - it makes sense to use it if you're maintaining code written for it, but for new projects please use something more modern. You'll save yourself a lot of unnecessary headaches (there's enough headache inducing stuff in C++ that you shouldn't pile it on with VC6 if you don't have to). – Michael Burr Jan 25 '10 at 08:16
  • Yes, I'm maintaining old code so I thought of trying to compile just in this environment. Unfortunately, it's not working. I'll just have to try it on other compilers I guess. – jasonline Jan 25 '10 at 08:23
  • Or maybe just live with the dtor being public if you have to use VC6. – Michael Burr Jan 25 '10 at 08:26
2

I'm no C++ or VC expert, but your example looks similar to the one described on this page ... which the author calls a compiler bug.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • @Stephen: Thanks for the info. Your link provides a list of search results. I browsed over and I think it's this one - http://www.codeguru.com/forum/archive/index.php/t-236067.html. – jasonline Jan 25 '10 at 10:07
0

Personally i haven't put destructors in my singletons unless i am using a template singleton class, but then i make them protected.

template<class T>
class Singleton
{
public:
    static T &GetInstance( void )
    {
        static T obj;
        return obj;
    }

    static T *GetInstancePtr( void )
    {
        return &(GetInstance());
    }

protected:
    virtual ~Singleton(){};
    Singleton(){};

};

then write my class as

class LogWriter : public Singleton<LogWriter>
{
friend class Singleton<LogWriter>;
}
Georg Fritzsche
  • 97,545
  • 26
  • 194
  • 236
Craig
  • 1,199
  • 1
  • 13
  • 27
  • @Craig: Thanks for sharing. I actually need the destructor to do some cleanup though. That's why. If I make it as protected, I get the same error saying cannot access protected member declared in... – jasonline Jan 25 '10 at 07:58
-1
class Singleton
{
     static Singleton *pInstance = NULL;  
     Singleton(){};

     public:

    static Singleton * GetInstance() {

          if(!pInstance)
          {
               pInstance = new Singleton();
          }
          return pInstance;
     }

     static void  RemoveInstance() {

          if(pInstance)
          {
               delete pInstance;
               pInstance = NULL;
          }

     }
};
Ashish
  • 8,441
  • 12
  • 55
  • 92
  • @Mac: Thanks for sharing. But with this one, you actually need the caller to call RemoveInstance() right for the cleanup. I was thinking of an implementation where you don't need to ask the caller to call a cleanup function. – jasonline Jan 25 '10 at 08:06
  • This is the standard anti-pattern for writing a singleton. Please learn the correct (or should I say better ways to do it). Scott Myers has some really good examples. – Martin York Jan 25 '10 at 09:31