Question

I'm writing a wrapper around a class library that I have no control over. In this library is a class (let's call it Target) that I want to ensure is only instantiated once, but it is not, in itself, a singleton. I thought of using the Singleton-Factory pattern like so:

internal sealed class SingletonFactory
{
    private static readonly SingletonFactory manager = new SingletonFactory();

    private readonly Target target;

    private static SingletonFactory() { }

    private SingletonFactory()
    {
        target = new Target();
        target.Init("foo");
    }

    internal static SingletonFactory Instance
    {
        get { return manager; }
    }

    internal Target Target
    {
        get { return target; }
    }
}

I can then do:

var targetInstance = SingletonFactory.Instance.Target;

I then thought of simplifying this by making the Factory completely static like this:

internal static class StaticFactory
{
    private static readonly Target target;

    private static StaticFactory()
    {
        target = new Target();
        target.Init("foo");
    }

    internal static Target Target 
    {
        get { return target; }
    }
}

and access to the target instance becomes:

var targetInstance StaticFactory.Target;

I'm pretty sure this StaticFactory is thread-safe, and provides global access to a single instance of the target class. Is there anything wrong with this that I haven't thought of?

Was it helpful?

Solution

I'm not certain that your constructor is really thread-safe, since it technically could be accessed from different threads at the same time. You could lock on a private static readonly object Lock = new object(); to enforce thread safety there.

If we are talking about C# 4, you might also want to look at Lazy<T> here http://msdn.microsoft.com/de-de/library/ee792409.aspx. It supports a thread-safe creation mode LazyThreadSafetyMode that allows you to find a tradeoff between safety and performance.

Sidenote: In the end, not using a static class might be a better design decision as you prevent an architecturally invisible dependency and gain the ability to swap implementations. Working with abstract factories addresses that (and also allows a nice unit testing experience).

OTHER TIPS

Same thing really, except yours had the private keyword on the static constructor, which is disallowed.

internal static class StaticFactory
{
    public static Target Target = new Target();

    static StaticFactory()
    {
       Target.Init("foo");
    }
}

You could get fancy, and shove it all into a Lazy:

public static Lazy<Target> Target =
      new Lazy<Target>(() => { var t = new Target(); t.Init(""); return t; });

You could also establish a facade, which would give you the same semantics as Target, but keep it as a single instance. It also gives you room to play with where and when you initialize the Target object.

public class TargetFacade
{
   private static Target _target = new Target();

   static StaticFactory()
   {
      _target.Init("foo");
   }

   //Wrap Target's methods here.
   public int Score { get { return _target.Score } }; 
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top