Singleton pattern doubt

[Vivek Hingorani]

I read about singleton and tried implementing the same via this piece of code:

public class Singleton {
	  private static Singleton singleInstance;
	    private Singleton() {}
	  public static Singleton getSingleInstance() {
	    if (singleInstance == null) {
	      synchronized (Singleton.class) {
	        if (singleInstance == null) {
	          singleInstance = new Singleton();
	        }
	      }
	    }
	    return singleInstance;
	  }
  
  @Override
protected Object clone() throws CloneNotSupportedException {
	// TODO Auto-generated method stub
	throw new CloneNotSupportedException();
}

Then wrote a main method in the same class and tried testing it:

Singleton s1=new Singleton();
	  Singleton s2=new Singleton();
	  System.out.println(s1.hashCode());
	  System.out.println(s2.hashCode());
	  System.out.println(s1.equals(s2));
  }

As its the same class I was able to create a object with new operator. Here both the Objects are different and even equals prints false. Is it not the correct singleton implementation?

[Campbell Ritchie]

Whether you should be using singletons in the first place is doubtful, but …

Where did you find that? It is a pretty poor implementation. And putting your main method in the same class is another bit of dubious design.
You are using double‑checked locking, and everybody knows double‑checked locking doesn’t work. … and they are mistaken; it does work in Java5+.
I think Joshua Bloch (Effective Java™, but I don’t have my copy to hand at present) recommends you implement a singleton like this:-

public final class Singleton
/* final is redundant; you can't extend a class with a private constructor */
{
   private static final Singleton INSTANCE = new Singleton();

   private Singleton()
   {
      super();
   }

   public static Singleton getInstance()
   {
      return INSTANCE;
   }
// ... etc etc
}

I think I have got that right; please check.

No need to do anything with clone(). If you don’t implement Cloneable, you can’t use clone() anyway. Similarly, don’t implement Serializable.

[Vivek Hingorani]

Agreed double locking doesnt work.. But Whats the prob in having main method in same class?

That is breaking my singleton and hence the doubt.

[Winston Gutkowski]

Vivek Hingorani wrote:Is it not the correct singleton implementation?

Not really.

1. SingletonsAreEvil. You may also find this article useful.
2. Lazily instantiated singletons (which appears to be what you're trying to do) are doubly evil.
3. If, in spite of all the evidence, you absolutely feel you must have a singleton:

public enum Singleton {
   INSTANCE;
}

is probably the easiest way to define it.

Winston

[Joanne Neal]

Vivek Hingorani wrote:But Whats the prob in having main method in same class?

If your constructor is private then the only way to create an instance of that class is from within the class itself.
In your class there are two methods that create instances of your class

getSingleInstance which only allows one instance to be created

main which creates multiple instances

Therefore if you want to make sure only one instance is created, get rid of your main method.

[Naveen Bachu]

This implementation seems to be very poor, we create Singleton class so that no one should be able to create two instances Note: this rule is also applicable for the person who is writing this class

Here is the example which explains about implementation of Singleton class: Best Way to Create Singleton Class

[Junilu Lacar]

Naveen Bachu wrote:Here is the example which explains about implementation of Singleton class: Best Way to Create Singleton Class

That's not a good example. In fact, you should avoid implementing a Singleton that way because it is not thread-safe. See Campbell's and Winston's examples instead.

[Naveen Bachu]

This will still work by adding synchronized block

[Junilu Lacar]

It's still mislabeled as "the best way" at best, falsely advertised at worst. Don't use that method, even with synchronization. There are better ways.

[Campbell Ritchie]

It is surprising how many websites there are which show poor examples of Java code and describe them as the best way.

[Naveen Bachu]

It's still mislabeled as "the best way" at best, falsely advertised at worst. Don't use that method, even with synchronization. There are better ways.

I didn't get what is wrong with the code, except synchronization, Can you guide us to any best example to compare and improve our knowledge?

[Junilu Lacar]

Naveen Bachu wrote:I didn't get what is wrong with the code, except synchronization, Can you guide us to any best example to compare and improve our knowledge?

That guidance has already been given. Read through the thread again.

See also this wikipedia entry on singleton pattern but read it carefully so you know which ones are examples of bad implementations vs which ones are good implementations.