0

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?

Andrew Cooper
  • 32,176
  • 5
  • 81
  • 116
  • You mention that you do not have any control over `Target`. How will you guarantee that someone does not instantiate `Target` outside of your factory, especially if `Target` is public? – Tung Mar 16 '12 at 03:57
  • The wrapper library I'm writing will hide the assembly that `Target` comes from. This is an internal company project so the usage of the library will be fairly small. – Andrew Cooper Mar 16 '12 at 05:25

2 Answers2

1

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 } }; 
}
Ritch Melton
  • 11,498
  • 4
  • 41
  • 54
1

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).

sunside
  • 8,069
  • 9
  • 51
  • 74
  • Thread safe singletons in C# have been discussed quite a bit. Here's a decent walkthrough. http://www.yoda.arachsys.com/csharp/singleton.html Its easier than you think. – Ritch Melton Mar 16 '12 at 05:09
  • That actually looks less easy than I thought. :D Interesting read anyway! – sunside Mar 16 '12 at 05:13
  • Oh, I think the lock-free version is pretty simple. – Ritch Melton Mar 16 '12 at 05:14
  • Absolutely, I just find it harder to grok than the locked version because it is not obvious that this is thread safe. The two empty constructors add to this. – sunside Mar 16 '12 at 05:19