Reputation: 5469
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
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 FileShareAccessFactory
s.
Other improvements you must make if you keep your code to enforce just one instance:
Upvotes: 0
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
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