Singleton problem

[abhi_elementx]

In this code :

#include <iostream>
using namespace std;

class Singleton{
	private:
		Singleton(){}
	public:
		static Singleton* getInstance();
		void myFunction(){
				cout<<"\nmyFunction() called..";
   	    }
};


Singleton* Singleton :: getInstance(){
	static Singleton* obj = NULL;
    if(obj == NULL){
    	cout<<"\nCreating new instance...";
    	obj = new Singleton();
    	cout<<"\nReturning instance...";
    }

    else{
    	cout<<"\nan old instance already exists...";
        cout<<"\nReturning existing instance...";
    }
    return obj;
}

int main(){
		Singleton* myobj = Singleton :: getInstance();
		myobj->myFunction();

		Singleton* myobj2 = Singleton :: getInstance();
				myobj2->myFunction();
}

How many times does this line:

static Singleton* obj = NULL;

execute?
Thnk you.

[MrSpigot]

It's only executed once, so that class would work as a singleton.
An alternative technique which avoids the need to clean up the memory allocation is:

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

Pros and cons to each method.

[abhi_elementx]

But I heard that your code is not good for multithreaded program.
Justify if I m wrong.

[StuXYZ]

Ok it is a singleton, so there are several methods, but teh stuff above doesn't look 100% ok.

First : if you are going to use a singleton you want to be 100% certain that there are no repeats.

That means that if you want to use the static pointer version you want this.

class Singleton
{
  private:

    Singleton();
    Singleton& opertaor=(const Singleton);
    Singleton(const Singleton);

 public:

    ~Singleton();        

    Singleton* Instance(); 
};

Note that you MUST put the copy constructor and assignment operator in the class definition because if you don't you get a default version AND that is public.

The advantage of the pointer version, is that creation is only needed in the case that you actually need it, and it naturally allows creatiion/deletion (with a bit of modification), as well as one of serveral models etc.

In multi-threaded case problem, it is the creation code that needs to be thread protected. Just lock on entry to creation.

[ArkM]

static Singleton* obj = 0; is a local static variable definition (declaration + initialization). An implementation is permitted to perform so called early initialization of the obj (before Singleton::getInstance call) or to perform it at the first entry to the getInstance. Avoid recursive initialization of local variables with static storage duration (it's not your case):

int f()
{
  static int v = f();
  return 0;
}

It's forbidden (undefined program behavior).

[MrSpigot]

But I heard that your code is not good for multithreaded program.
Justify if I m wrong.

You're correct. The Meyers singleton (can't quite claim it as my own code) isn't guaranteed threadsafe in that form but you can apply locking to this just as easily as to the static pointer implementation.

Thanks to ArkM for mentioning that early initialisation may initialise local statics before the function is entered. Didn't know that, and it's those hidden initialisation bugs that can take days to track down. Doesn't prevent use of The Meyers singleton, but it may influence your choice, depending on the circumstances.