March 31, 2008
@ 10:00 AM
While I personally don't like the Singleton pattern too much (it is essentially an OO mask for Global Variables, it makes it harder to unit test etc.), I still need to use them now and then. Anyway, I saw a post by Jack Altiere about "Using the Singleton Pattern" in .NET and since it presents an implementation that leaves a lot to be desired I decided to comment on that

For one, he presents a solution which locks every time the singleton is accessed (see below). The reason this is flawed is that singletons are instantiated once, but they are accessed a lot of times and with this implementation you pay every time you try to access the singleton

//bad implementation - don't use
//also note some initialization etc. is omitted for brevity
public static Foo Instance
{
get
{
// This lock allows thread safety.
lock (_mutex)
{
if (_instance == null)
_instance = new Foo();
return _instance;
}
}
}

There's a better "standard" way to do that which basically means you check if the the instance is null, lock, check again (in case more than one thread got past the first check ) and only then instantiate (you'd also need to mark the instance volatile in this case)
.NET has a better way to create thread safe singletons by merely using a static readonly declaration as in:
public  sealed class singleton
{
public static readonly MySigleton Instance = new MySingleton()
}



This implementation is not as lazy as the implementations above - but it is thread safe an efficient (you can get more laziness if you use a nested class as described here)

But that's not all- see, one of the problems of the Singleton pattern (besides the ones mentioned above) is that it is also a violation of the Single Responsibility Principle (SRP see OO primer). You can solve this problem if you use Generics and create something like C++'s template based Singleton class:
namespace ConsoleApplication5
{
class Singleton<T> where T : new()
{
private static readonly T inst = new T();

public static T Instance
{
get { return inst; }
}
}
}


and then use that simply by doing something like Singleton<MyClass>.Instance
[update]
Eran points out that Jack's code is not only inefficient but also not thread safe (see Eran's post on the subject) - basically you need to make the instance variable volatile just like in the double check scenario

Reshef points to the "Static Gateway Pattern" as an alternate  way to get the singleton effect. While we are talking about other possibilities, I should have probably mentioned that the usual way I handle situation where I need a single instance is instantiating one instance on the main/loader thread and then just using dependency injection... :)


 
Tags: .NET | Design

Monday, March 31, 2008 9:25:07 AM (GMT Standard Time, UTC+00:00)
If using the public static readonly implementation, consider extracting an interface to improve decoupling and testability for your consumers.

e.g
public sealed class MySingleton : IMySingleton

{ public static readonly IMySingleton Instance = new MySingleton()

private MySingleton()
{}

}
S.Youth
Monday, March 31, 2008 10:02:13 AM (GMT Standard Time, UTC+00:00)
I don't like singletons either...

but

U can take a look here for deep analysis of singletons in .net: http://www.yoda.arachsys.com/csharp/singleton.html

It also suggests how to achieve lazyness using a static field.

And this strategy: http://www.jpboodhoo.com/blog/TheStaticGatewayPattern.aspx

lets u use singleton (static in this case) without sacrificing the ability to test.
Monday, March 31, 2008 10:46:25 AM (GMT Standard Time, UTC+00:00)
Hi Ronen,
There is a thread-safety problem with the first implementation in your post.
I've written a detailed explanation about it here:

http://www.ekampf.com/blog/2007/07/15/WhatsWrongWithThisCode1Discussion.aspx


Regards,
Eran
Monday, March 31, 2008 11:19:29 AM (GMT Standard Time, UTC+00:00)
@Eran, Reshef: I updated the post

@S.Youth : that doesn't help much since you access the singleton using the Class (Singleton.GetInstance.) it is better to use dependency injection or a singleton that has members which are the interfaces (sort of like a directory of singletons)

Arnon
Monday, March 31, 2008 11:55:12 AM (GMT Standard Time, UTC+00:00)
Arnon,

? I think it does help because, your client code simply writes against the interface, and has no knowledge (other than in this case via the obvious variable name used) that it is consuming a singleton, and it is fully testable.

e.g

public class SomeClient
{
private IMySingleton _someSingleton;

public SomeClient(IMySingleton someSingleton)
{
_someSingleton = someSingleton;
}

public void DoSomeClientStuff()
{
_someSingleton.DoSomeMethodOnSingleton;
}

}
S.Youth
Monday, March 31, 2008 12:07:35 PM (GMT Standard Time, UTC+00:00)
@S. Young
What you are doing is using dependency injection. In this case you don't really need the class to be singleton

Arnon
Monday, March 31, 2008 12:12:34 PM (GMT Standard Time, UTC+00:00)
Hi Arnon,

By the way, it is important to note that the Instance in the sample in the first comment that I added, is of type IMySingleton.

That is, public static readonly IMySingleton Instance = new MySingleton()

SY
S.Youth
Monday, March 31, 2008 7:16:44 PM (GMT Standard Time, UTC+00:00)
There are risks with the Generics approach:
http://www.dotmad.net/2008/03/generic-singleton-risks.html
Tuesday, April 01, 2008 8:33:01 AM (GMT Standard Time, UTC+00:00)
I guess you can use reflection to avoid the requirement for a public constructor in your Singleton template.
Kind of ugly but should do the trick...
Tuesday, April 01, 2008 1:15:10 PM (GMT Standard Time, UTC+00:00)
But the fact you write your class seperately from the template makes it easier to make mistakes to re-enable the public constructor.
Tuesday, April 22, 2008 1:09:20 PM (GMT Standard Time, UTC+00:00)
Have you tried the double checking lock pattern instead eg....

<blockquote cite="">
public static Foo Instance
{
get
{
if (_instance == null)
{
lock (_mutex)
{
if (_instance == null)
{
_instance = new Foo();
}
}
}

return _instance;
}
}
</blockquote>

It may look daft, but it works quite well, we used this for a scheduler that ran inside the ASP.NET context (we couldn't use a Windows Service for various security reasons).

I personally don't like 'private static readonly' method outlined in this article because it doesn't let you set properties and configure the singleton instance, say for pulling out values from thw web.config.
Tuesday, April 22, 2008 9:04:54 PM (GMT Standard Time, UTC+00:00)
Hi Dominic
I am aware of the double check approach (note that you need to make the instance variable volatile or it isn't really thread safe)
I find the static readonly way simpler and less prone to errors (such as I mentioned above) and as I mentioned in the article I rather use Dependency Injection and control the lifespan of the objects externally (e.g. using facilities of the IoC controller )

Arnon
Comments are closed.