James Ko
James Ko

Reputation: 34609

Using optional singletons in OOP?

I'm writing a PCL in .NET and I have a wrapper class around HttpClient that loads an HtmlAgilityPack.HtmlDocument from a URI in multiple different methods. It is stateless, so I would really like to make it static, since in my opinion instantiating something with new gives the impression that it contains state. However, I have a couple of interfaces that I want it to inherit from, so it can't be static. This is where I thought of making it a singleton. Here are a few snippets from the code:

public class ConcurrentClient : IAsyncClient<HtmlDocument>
{
    private static readonly ConcurrentClient _Instance = new ConcurrentClient();

    private ConcurrentClient() { }

    public static ConcurrentClient Instance
    {
        get { return _Instance; }
    }

    public HtmlDocument LoadUri(string uri)
    {
        return LoadUriAsync(uri).Result;
    }

    // ...

    public async Task<HtmlDocument> LoadUriAsync(string uri, 
        Encoding e, NetworkCredential creds, Action<HtmlDocument> prehandler)
    {
        // ...
    }
}

I'm wondering, though, if I should change the beginning parts to this:

private static readonly ConcurrentClient _SharedInstance = new ConcurrentClient();

public static ConcurrentClient SharedInstance
{
    get { return _SharedInstance; }
}

The reason for this is I'm not that sure about using the Singleton pattern, mainly because I've rarely seen it in use in other libraries (maybe WinRT's Application.Current?), and I think it would encourage users of my PCL to write coupled code, since it's much easier to just call ConcurrentClient.Instance everywhere than it is to pass it in as a parameter.

However, I do want to encourage the use of a shared instance because excluding the reasons above, there's very little point in calling new ConcurrentClient() because all it does is create more memory overhead. Also, I can't think of a better way to implement inheritance with methods that don't really rely on state.

Upvotes: 1

Views: 126

Answers (1)

Philip Stuyck
Philip Stuyck

Reputation: 7467

Your Singleton already implements 2 interfaces. The question really is, where are the dependencies to this Singleton and why are they there ?
If the answer is that these dependencies are there because they need to get to the implementation of those interfaces then I would say that this is wrong.
The whole point of doing a SOLID design is to have dependencies towards interfaces and not towards any concrete implementation. So anyone who needs any of these 2 interfaces should be given those interfaces via dependency injection. So that means that the interfaces would be passed by their constructor or via an extra parameter in a method call, a strategy pattern, ...

Also see this : http://blogs.msdn.com/b/scottdensmore/archive/2004/05/25/140827.aspx

There can be a reason to make a singleton, but based on your explanation this is not that clear.

Investigate more of your time in using dependency injection. If you have that under control move one step further and investigate on how you can use an inversion of control container.

Also, it's easy to forget DI and passing around the object as a parameter when you can just access it by Singleton.Instance.

You are forgetting about unit testing. If you pass interfaces to your class constructors you can easily mock those interfaces and test your class functionality. With your singleton in place, your classes really need that singleton. Unit testing will be harder. Of course Instance is easy to access, it's a global and since people revert back to old habits of programming towards objects all the time, that is why it is so popular.

Upvotes: 2

Related Questions