Singleton shortened implementation

Question

I always see singletons implemented like this:

public class Singleton
{
    static Singleton instance;
    static object obj = new object();

    public static Singleton Instance
    {
        get
        {
            lock (obj)
            {
                if (instance == null)
                {
                    instance = new Singleton();
                }
                return instance;
            }
        }
    }

    protected Singleton() { }
}

Is there's something wrong with implementing it like this:

public class Singleton
{
    static readonly Singleton instance = new Singleton();
    public static Singleton Instance
    {
        get { return instance; }
    }

    protected Singleton() { }
}

? It gets lazy initialized on the very same moment as in the first implementation so I wonder why this isn't a popular solution? It should be also faster because there's no need for a condition, locking and the field is marked as readonly which will let the compiler to do some optimisations

Let's not talk about the singleton (anti)pattern itself, please

You might be interested in reading Jon Skeet's [Implementing the Singleton Pattern in C#](http://csharpindepth.com/Articles/General/Singleton.aspx), scroll down to "Fourth version - not quite as lazy, but thread-safe without using locks" — dcastro, Jan 15 '15 at 10:14
Its a matter of taste. There's a third pattern with `Lazy`, as well. Note that your first example should also have a check of nullity *before* entering the lock. — Yuval Itzchakov, Jan 15 '15 at 10:15
There's nothing wrong with implementing it like that (other than you should declare the backing field as `readonly`). But note that it's not as lazy as the first version since the object is created when a static method in the class is called, rather than when the property is accessed. — Matthew Watson, Jan 15 '15 at 10:17

score 7 · Accepted Answer · edited May 23 '17 at 11:57

The CLR will initialize the field upon the first use of that (or any other static) field. It promises to do so in a thread-safe manner.

The difference between your code is that the first bit of code supports thread-safe lazy initialization where the second doesn't. This means that when your code never accesses Singleton.Instance of the first code, no new Singleton() will ever be created. For the second class it will, as soon as you access Instance or (directly or indirect) any other static member of that class. Worse even - it may be initialized before that because you lack a static constructor.

Favoring shorter and more readable code, since .NET 4 you can use Lazy<T> to significantly shorten the first code block:

public class Singleton
{
    static readonly Lazy<Singleton> instance = 
        new Lazy<Singleton>(() => new Singleton());

    public static Singleton Instance
    {
        get { return instance.Value; }
    }

    static Singleton() { }
    private Singleton() { }
}

As Lazy<T> also promises thread-safety. This will ensure new Singleton() is only called once, and only when Singleton.Instance is actually used.

Worth noting that making the constructor `protected` will allow the class to be inherited and thus making it not a singleton. — Sriram Sakthivel, Jan 15 '15 at 11:06

Singleton shortened implementation

1 Answers1