Implementting Singleton Design Pattern [Please Suggest]

Can anyone identify the problem within this snippet of Java/C# code implementing Singleton Design Pattern.

Can someone find me the flaw in this implementation of this snippet?

class Singleton{
public static Singleton Instance() {
if (_instance == null)
_instance = new开发者_运维问答 Singleton();
return _instance;
}
protected Singleton() {}
private static Singleton _instance = null;
}

It's not thread-safe

http://csharpindepth.com/Articles/General/Singleton.aspx

As Kieren says, it's not thread-safe... and also you've allowed classes to derive from it, which means it's not really a singleton:

public class SingletonBasher : Singleton
{
}

Singleton x = Singleton.Instance();
Singleton y = new SingletonBasher();

It should be invalid for x and y to be different references and both of them to be non-null - it violates the concept of a singleton.

(And yes, I'd recommend my article on singleton implementations too :)

A better way to implement singleton (as Joshua Bloch's Effective Java) is to use enum:

enum Singleton {
    INSTANCE();
    private Singleton() {
       ....
    }
}

If you insist with your approach, you need to do three more things:

• make the constructor private (as Jon Skeet's suggestion)
• make _instance volatile
• double lock the Instance()

enum A { A; }

If you are in a multithreaded environment two different threads might enter the if block and then both of them will create a new instance. Add locking:

class Singleton{
    public static Singleton Instance() {
        if (_instance == null) {
            lock(typeof(Singleton)) {
                if (_instance == null) {
                    _instance = new Singleton();
                }
            }
        }
        return _instance;
    }
    protected Singleton() {}
    private static Singleton _instance = null;
}

All the above answers are correct.

In singleton design pattern you should not allow others to create instance for singleton class. You have to own your own rights. The other instance can request you for singleton's instance. So constructor should/must be private.

In your example you made it as protected, In this case if you extends the singleton class you have a possibility of creating an instance from other instance. This possibility should not be provided.

Make constructor as private in your code snippet, then its singleton class.

Its not thread-safe.

You can implement in a different way with pre-initialization:

private static Singleton _instance = new Singleton();

public static Singleton Instance()
{
    return _instance;
}

This method is thread-safe as you are only returning the instance in the method.