Is this singleton implementation correct and thread-safe?
class Class
{
public static readonly Class Instance;
static Class()
{
Instance = new Class();
}
private Class() {}
}
4 Answers
Technically, your version should work. However, I would not recommend exposing a public field within your Singleton class, and prefer using a Property (with a getter only). This will help future-proof your API if you need to make changes later. I also recommend sealing any singleton implementation, as subclassing a singleton class is almost always a bad idea and problematic.
I would, personally, use the following in C#, if you're targetting .NET 3.5 or earlier:
public sealed class Singleton
{
static readonly Singleton instance = new Singleton();
public static Singleton Instance
{
get
{
return instance;
}
}
static Singleton() { }
private Singleton() { }
}
If you're using .NET 4, you can make this even easier for yourself via Lazy<T>:
public sealed class Singleton
{
private static readonly Lazy<Singleton> instance = new Lazy<Singleton>( () => new Singleton() );
private Singleton() {}
public static Singleton Instance { get { return instance.Value; } }
}
The .NET 4 version also has the advantage of being fully lazy - even if your Singleton class has other static methods which are used prior to the access of the "Instance" property. You can do a fully-lazy .NET 3.5- version, as well, by using a private, nested class. Jon Skeet demonstrated this on his blog.
6 Comments
Andreas
I especially like the Lazy<L> solution.
Reed Copsey
@Andreas: Me too - it's also more "lazy" - since accessing a static method on the class won't cause instantiation - only the "Instance" property.
Ferdil
Just one thing: could I do it with auto properties? like this: public static Singleton Instance { get; private set }
Reed Copsey
@Ferdil: You ~could~ potentially do it with an auto-property and a static constructor, but I would just use a backing field. In this case, it's more efficient, and it's more obvious (and less code, especially with the Lazy<T> option)
Reed Copsey
@huseyint: It's very similar, though technically not exactly the same. Field initialization happens prior to the static ctor, so if there's code in the ctor, the field init happens first. In the above, it would work fine, though.
|
Yes. I would also make the class 'sealed' to avoid any future confusion.
Comments
Good discussion of how to do that is here:
1 Comment
Robert Harvey
Yes, but what about the OP's version?
You should do the initialization in the variable declaration:
public static readonly Class Instance = new Class();
4 Comments
Joe
It delays construction of Instance until the first reference of it, which may make a difference. Plus to me it's a cleaner read.
Reed Copsey
@Joe: It does NOT delay construction until the first reference. In fact, you must leave the static constructor in place or the class will be marked beforefieldinit, which may actually cause it to be instantiated even earlier (though I prefer using the inline constructor as well).
Joe
@Reed: msdn.microsoft.com/en-us/library/ff650316.aspx states that it DOES delay construction (under "Static Initialization"). Though, that's not an exact equivalent of what I posted which may make all the difference.
Reed Copsey
@Joe: It delays construction until the TYPE is initialized, which means the first time you use "Class", "Instance" will get initialized. This is exactly the same as putting it in the static constructor, in terms of timing. However, without a constructor, your class will get tagged beforefieldinit, which allows the runtime to initialize even earlier. See Jon Skeet's post on singletons (referenced in my answer) for details on beforefieldinit, if you want more info.
static Class()declaration is for the static constructor which only gets called once. The call to 'new Class()' hits the 'private Class()' instance constructor.