Simple Singleton Pattern in C#

Member variable should be private. Not multithread safe. You have to hide member variable and should implement locking mechanism.

bad design, to simple

Public field????? That should be private.

Where is a lazy initialization?

Reason for my vote of 2
This alternative is not 'lazyloading'.

As far as "Lazy loading" goes, in terms of when the Singleton instance is created there is unlikely to be any noticeable difference between:

Option 1:

C#

sealed class Singleton
{
    private static Singleton m_Instance = new Singleton();

    // Prevent instance creation from other classes
    private Singleton() { }

    public static Singleton Instance { get { return m_Instance; } }

    // Non-static instance members here...
}

and Option 2:

C#

using System.Threading;
sealed class Singleton
{
    private static readonly Singleton m_Instance;

    // Prevent instance creation from other classes
    private Singleton() { }

    public static Singleton Instance
    {
        get
        {
            if (m_Instance == null)
            {
                Singleton temp = new Singleton();
                Interlocked.CompareExchange(ref m_Instance, temp, null);
            }
            return m_Instance;
        }
    }

    // Non-static instance members here...
}

In both cases, m_Instance gets created the first time you call "Instance" - in the first case it is just before executing the function, in the latter it is done within the method, if it hasn't already been created.

Note that with the second option you have to take care of thread-safety yourself. In my example above I've used Interlocked.CompareExchange to achieve this, after checking if the object is null. (This again is not the only way to do this and has its pros and cons - for example, in this case it would still theoretically be possible to have more than one Singleton object created, albeit temporarily, if "Instance" was called in very quick succession from several threads and especially if the constructor of the class was complicated; however, only one of them would ever be returned to any caller).

Therefore the second option necessarily has some extra overhead, which option 1 does not have since the static member needs to be created anyway and the CLR itself takes care of any race conditions. Plus for option 2 every time the Instance member is called thereafter it has to check whether m_Instance is null each time.

If you had some other static members that you could call on the class, then use of the second option may be more appropriate, since in the first option m_Instance would be created as soon as any of them was called. (However, that may be a desired side-effect, so don't rule it out because of that).

There are other pros and cons with each option, but I note there are already so many articles on this I don't intend to go into them in this reply.

Regards,
Ian.

modified 11-Apr-13 13:20pm.

An exception thrown inside Singletin's .ctor will be thrown on type initialization (instead of first access to Instance property and even if you will never use it) and wrapped into TypeInitializationException. Alternative 6 solves that without any performance loss.

It is wrong that an exception might be thrown "even if never used". Because the field is static, it will be created when the class is first accessed. So an exception thrown by the constructor would still only occur at the first call to "instance" (assuming it is the only publicly accessible member of the class). See my reply to Kabwla.Phone, above.

Try to debug this:

C#

sealed class Singleton
{
    private static readonly Singleton m_Instance = new Singleton();

    // Prevent instance creation from other classes
    private Singleton() {
        throw new Exception();
    }

    public static Singleton Instance { get { return m_Instance; } }

    // Non-static instance members here...
}

And compare with:

C#

internal sealed class Singleton
{
    private static readonly object SyncRoot = new object();
    private static volatile Singleton m_Instance;
    // Prevent instance creation from other classes
    private Singleton()
    {
        throw new Exception();
    }

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

    // Non-static instance members here...
}

Where is debugger located upon throw?
What type of exception is thrown?
Can you program continue execution correctly?

My answer was in response to your assertion:

Quote:
An exception thrown inside Singletin's .ctor will be thrown on type initialization (instead of first access to Instance property and even if you will never use it)

This is not quite correct! If you never call "Instance", the exception will never be thrown, even if you explicitly create one like this (because the type will not ever get initialised). In other words, the exception would still get thrown the first time the class is required, i.e. in the former case, "just before" the call to Instance is made.

You are correct that if there should be an error in the constructor then the former pattern can be harder to debug. However, I wonder if you've forgotten that an exception should be just that - exceptional! It may well be that if the instance class fails to be instantiated that the application could not continue anyway, so allowing the application to terminate can be the best option. And if you have a very simple class that should not throw an exception, then the former pattern may be better (e.g. it gets around the need for your own locking mechanism in the Instance method, and the overhead involved in that. Although of course, you mustn't forget that you may need to allow for locking mechanisms in the class's members!)

This is why I will not say that one pattern is better than the other. It's all about design.

As I said in my reply to Kabwla.Phone[^], there are pros and cons to each pattern. I nearly mentioned the problem of the difficulty to debug, but must have decided not to do so as my responses were all getting rather long.

Regards,
Ian.

Reason for my vote of 5
the simplest and because of it the best option

It is one option. It is not necessarily the "best". There may be times to use this option, but there may be times to use the option of creating the single instance object from within the "Instance" property.

Also, since this code is supposed to be an example, I'm afraid it is not a very good one:
1) Note that the single instance object (m_Instance in this case) should be private.
2) You should not allow inheritance from a single instance class, so it should be sealed.

C#

sealed class Singleton {
    private static readonly Singleton m_Instance = new Singleton();

    // Prevent instance creation from other classes
    private Singleton() { }

    public static Singleton Instance { get { return m_Instance; } }

    // Non-static instance members here...
}

Regards,
Ian.

The field should probably be private.

Simple Singleton Pattern in C#

License

Comments and Discussions