Expert: Zlatko - 3/19/2011

Question
Hi Zlatko,
        Please go through the following code:
/**************************************************************/
#include "iostream"
using namespace std;
class Singleton{

private:
  static Singleton* pInstance;
  Singleton();
  ~Singleton();
public:
  static Singleton* getInstance();
  void destroy();
};

Singleton* Singleton::pInstance = 0;
void Singleton::destroy()
{
  delete pInstance;
}

Singleton::Singleton()
{
  cout<<"Instance Created"<<endl;
}
Singleton::~Singleton()
{
  cout<<"Destructor Called"<<endl;
}
Singleton* Singleton::getInstance()
{
  if(pInstance == 0)
  {
     pInstance = new Singleton();
  }
  return pInstance;
}
/***************************************************************/
      
   My question is is this correct way of writing Singleton class destructor code for Single ton class.  
   
Thanks in advance,
Bhupal.

Answer
Hello Bhupal

I think the destroy should also be static because you should not need an actual instance of Singleton in order to access the static pInstance. I know it does not make sense to call destroy if an instance has not been created, but I think the destroy method should match the create method, and be static.

In the destroy method you should check if pInstance is NULL (0) and do the delete only when it is not NULL. After the delete, pInstance should be set to NULL.

Since there should be only one instance of Singleton, it should have a privately declared assignment operator and copy constructor. You do not need an implementation for them

It is correct to make the constructor and destructor private so that the user cannot make another instance or delete any instance.

The code with my suggestions is below.

Best regards
Zlatko


#include <iostream>
using namespace std;

class Singleton
{

private:
   static Singleton* pInstance;
   Singleton(const Singleton&); // keep unimplemented
   void operator=(const Singleton&); // keep unimplemented
   Singleton();
   ~Singleton();

public:
   static Singleton* getInstance();
   static void destroy();
};

Singleton* Singleton::pInstance = 0;

void Singleton::destroy()
{
   if (pInstance != NULL) delete pInstance;
   pInstance = NULL;
}

Singleton::Singleton()
{
   cout<<"Instance Created"<<endl;
}

Singleton::~Singleton()
{
   cout<<"Destructor Called"<<endl;
}

Singleton* Singleton::getInstance()
{
   if(pInstance == 0)
   {
       pInstance = new Singleton();
   }
   return pInstance;
}