Is this singleton implementation correct and thread-safe?
class Class
{
public static readonly Class Instance;
static Class()
{
Instance = new Class();
}
private Class() {}
}
views:
187answers:
4Yes, but what about the OP's version?
Robert Harvey
2010-06-28 20:52:37
+2
A:
Yes. I would also make the class 'sealed' to avoid any future confusion.
RichAmberale
2010-06-28 20:51:05
+1
A:
You should do the initialization in the variable declaration:
public static readonly Class Instance = new Class();
Joe
2010-06-28 20:52:09
It delays construction of Instance until the first reference of it, which may make a difference. Plus to me it's a cleaner read.
Joe
2010-06-28 20:58:43
@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).
Reed Copsey
2010-06-28 21:00:21
@Reed: http://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.
Joe
2010-06-28 22:46:22
@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.
Reed Copsey
2010-06-28 22:49:06
+6
A:
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.
Reed Copsey
2010-06-28 20:52:22
@Andreas: Me too - it's also more "lazy" - since accessing a static method on the class won't cause instantiation - only the "Instance" property.
Reed Copsey
2010-06-28 20:57:23
Just one thing: could I do it with auto properties?like this:public static Singleton Instance { get; private set }
Ferdil
2010-06-29 06:23:16
@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
2010-06-29 15:49:07