CODEWITHSUNDEEP
c#singleton
aleczandru
aleczandru

Reputation: 5469

Singleton implementation check

Hi I am interested in implementing a method that returns an singleton object.I have created an implementation based on an example found on MSDN but I am not really sure if my implementation is corect.

The code runs just fine but I am not sure how to check if it's the same object instance.

Here is my code:

 public class FileShareAccessFactory : IFileShareAccessFactory
{
    private volatile static IFileShareAccess m_fileShareAccess;
    private static object m_SyncRoot = new object();

    public IFileShareAccess GetFileShareAccessInstance(IContextFactory contextFactory, ILogger logger)
    {
        if (m_fileShareAccess == null)
        {
            lock (m_SyncRoot)
            {
                if (m_fileShareAccess == null)
                {
                    m_fileShareAccess = new FileShareAccess(contextFactory, logger);
                }
            }
        }
        return m_fileShareAccess;
    }
}

Upvotes: 2

Views: 490

Answers (3)

user2586804
user2586804

Reputation: 321

I now notice you have dependencies in your constructor, if this really is a singleton, then they also must be singletons. If not, then you want to think again about this as a singleton.

You may want to look at dependency injection such as Unity. This way you can register this class with ContainerControlledLifetimeManager to enforce a singleton on both the context factory, logger and this object. It allows for flexibility later where you may have more context factories thus need more FileShareAccessFactorys.

Other improvements you must make if you keep your code to enforce just one instance:

  1. Make the constructor private.
  2. Seal the class - or a descended class could be constructed.

Upvotes: 0

Alex Siepman
Alex Siepman

Reputation: 2608

Use object.ReferenceEquals() to check if both objects are the same instance.

For example:

    [TestMethod]
    public void TestGetFileShareAccessInstance()
    {
        var first = FileShareAccessFactory.GetFileShareAccessInstance(contextFactory, logger);
        var second = FileShareAccessFactory.GetFileShareAccessInstance(contextFactory, logger);

        Assert.IsTrue(object.ReferenceEquals(first, second));
    }

Upvotes: 0

Marc Gravell
Marc Gravell

Reputation: 1064324

As double-checked implementations go, yes - that'll work ok. It doesn't even need the volatile, since the synchronized double-check will deal with any small number of "it was a false null read". Personally, I'd be more concerned with the fact that it doesn't seem to respect the API - i.e. if I ask if for an instance specifying a particular context-factory and logger, it actually gives me something that used an unrelated context-factory and logger. Frankly, there are also IoC/DI containers which you could simply offload this to.

Upvotes: 6

Related Questions