7
\$\begingroup\$

What do you think about this for a generic singleton?

using System;
using System.Reflection;

/* Use like this
public class Highlander : Singleton<Highlander>
{
    private Highlander()
    {
        Console.WriteLine("There can be only one...");
    }
}*/

public class Singleton<T> where T : class
{
    private static T instance;
    private static object initLock = new object();

    public static T GetInstance()
    {
        if (instance == null)
            CreateInstance();

        return instance;
    }

    private static void CreateInstance()
    {
        lock (initLock)
        {
            if (instance == null)
            {
                Type t = typeof(T);

                // Ensure there are no public constructors...
                ConstructorInfo[] ctors = t.GetConstructors();
                if (ctors.Length > 0)
                {
                   throw new InvalidOperationException(String.Format("
                       {0} has at least one accesible ctor making it impossible
                       to enforce singleton behaviour", t.Name));
                }

                // Create an instance via the private constructor
                instance = (T)Activator.CreateInstance(t, true);
            }
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ So how do you stop someone from using the type T directly and creating multiple? \$\endgroup\$ – Jesse C. Slicer Apr 1 '12 at 21:28
  • \$\begingroup\$ Because there are no contructors on Type T, you cannot construct it normally. You have to go via the Singleton<T> template. \$\endgroup\$ – Simon Hughes Apr 1 '12 at 22:36
  • 2
    \$\begingroup\$ Seems pretty sweet to me. I do prefer using properties for this that's just a personal preference i.e. public static Instance { get; } vs GetInstance(). Would making the Singleton class abstract as well help ensure it's used in a inherited manner if that's the intention? \$\endgroup\$ – dreza Apr 2 '12 at 1:07
  • \$\begingroup\$ Superficial cosmetic comment: I think it is better to have something like string exceptionMessage = String.Format(\n"{0}...behaviour.",\nt.Name); and then throw new InvalidOperationException(exceptionMessage); right below. I also would use named arguments when calling Activator.CreateInstance - documentation without comments. I also prefer to always have braces along with if - this is the StyleCop way, but it is up to you. \$\endgroup\$ – Leonid Apr 2 '12 at 2:47
  • \$\begingroup\$ @LokiAstari Simon is using an overload of CreateInstance that can use a private constructor to create an instance. Notice the second parameter: CreateInstance(typeof(T), true); \$\endgroup\$ – Cristian Lupascu Apr 2 '12 at 13:45
5
\$\begingroup\$

Since we have .NET 4, we could make use of Lazy<T>

public class Singleton<T> where T : class, new()
{
    private Singleton() {}

    private static readonly Lazy<T> instance = new Lazy<T>(() => new T());

    public static T Instance { get { return instance.Value; } } 
}
\$\endgroup\$
3
\$\begingroup\$

From my tests you need to add the following code. The singleton class needs a private constructor to force the use of GetInstance().

private Singleton(){return;}
\$\endgroup\$
2
\$\begingroup\$

Your solution works and I think the resulting singleton declarations are quite elegant.

Having a singleton class declared as

public class Highlander : Singleton<Highlander>
{
    private Highlander()
    {
        Console.WriteLine("There can be only one...");
    }
}

is both concise and expressive.

The only problem I see with this implementation is in scenarios where performance is very important.

Due to the use of locking to ensure thread safety, it incurs a performance penalty that could be avoided. Please take a look at Jon Skeet's excellent article about singletons in C# for more details (more precisely the fourth example).

Also, there's one more subtle issue that affects performance. If there are many singleton classes defined using this mechanism, all calls that get an instance of any of them will wait on the same lock object (the initLock instance is shared by all these classes).

\$\endgroup\$
  • \$\begingroup\$ Last paragraph is actually incorrect - erased. \$\endgroup\$ – Cristian Lupascu Apr 2 '12 at 14:27
1
\$\begingroup\$

I'm not a fan of this design, mostly because it simply defers requirements to descendants. There's not that much involved in building a singleton, and my personal preference is still to build them from scratch, and dealing with the little gotchas there and then, instead of remembering to do them in a descendant class of a singleton (and even the phrase "a descendant of a singleton" has a very bad taste). YMMV, of course.

As far as your implementation is concerned: In the current setup, it's perfectly OK for the Highlander class to have internal or protected constructors and still pass the validation. This in turn allows new descendants of Highlander, which will, in effect lead to "multitons" (infinitons?).

\$\endgroup\$
0
\$\begingroup\$

So aside from the concerns with the singleton pattern as a whole (I'm not a fan, but I digress).

You're type checking is at runtime. I understand there's not a good way to enforce this at compile time right now. This would almost be handy here:

public class Singleton<T> where T: NOT new()

That would be the only problem I'd see. Runtime is a time for end users, and letting the end users know that a class can't be a singleton is not as helpful unfortunately.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.