Singleton with parameters

Question

I need a singleton class to be instantiated with some arguments. The way I'm doing it now is:

class SingletonExample
{
     private SingletonExample mInstance;
     //other members... 
     private SingletonExample()
     {

     } 
     public SingletonExample Instance
     {
         get
         {
              if (mInstance == null)
              {
                  throw new Exception("Object not created");
              }
              return mInstance;
         }
     }

     public void Create(string arg1, string arg2)
     {
         mInstance = new SingletonExample();
         mInstance.Arg1 = arg1;
         mInstance.ObjectCaller = new ObjectCaller(arg2);
         //etc... basically, create object...
     } 
}

The instance is created 'late', meaning I don't have all of the needed arguments on app startup.

In general I don't like forcing an ordering of method calls, but I don't see another way here. The IoC wouldn't resolve it either, since where I can register it in the container, I can also call Create()...

Do you consider this an OK scenario? Do you have some other idea?

edit: I know that what I wrote as an example it's not thread safe, thread-safe isn't part of the question

Solution 1

A Singleton with parameters smells fishy to me.

Consider whateva's answer and the following code:

Singleton x = Singleton.getInstance("hello", "world");
Singleton y = Singleton.getInstance("foo", "bar");

Obviously, x==y and y works with x's creation parameters, while y's creation parameters are simply ignored. Results are probably... confusing at least.

If you really, really fell like you have to do it, do it like this:

class SingletonExample
{
     private static SingletonExample mInstance;
     //other members... 
     private SingletonExample()
     {  // never used
        throw new Exception("WTF, who called this constructor?!?");
     }
     private SingletonExample(string arg1, string arg2)
     {
         mInstance.Arg1 = arg1;
         mInstance.ObjectCaller = new ObjectCaller(arg2);
         //etc... basically, create object...    
     } 
     public static SingletonExample Instance
     {
         get
         {
              if (mInstance == null)
              {
                  throw new Exception("Object not created");
              }
              return mInstance;
         }
     }

     public static void Create(string arg1, string arg2)
     {
         if (mInstance != null)
         {
             throw new Exception("Object already created");
         }
         mInstance = new SingletonExample(arg1, arg2);             
     } 
}

In a multithreading environment, add synchronisation to avoid race conditions.

Solution 2

Singleton is ugly but since user whateva can't be bothered to correct his own code...

public class Singleton 
{ 
    private static Singleton _instance = null; 

    private static Object _mutex = new Object();

    private Singleton(object arg1, object arg2) 
    { 
        // whatever
    } 

    public static Singleton GetInstance(object arg1, object arg2)
    { 
        if (_instance == null) 
        { 
          lock (_mutex) // now I can claim some form of thread safety...
          {
              if (_instance == null) 
              { 
                  _instance = new Singleton(arg1, arg2);
              }
          } 
        }

        return _instance;
    }
}  

Skeet blogged about this years ago I think, it's pretty reliable. No exceptions necessary, you aren't in the business of remembering what objects are supposed to be singletons and handling the fallout when you get it wrong.

Edit: the types aren't relevant use what you want, object is just used here for convenience.

Solution 3

Better answer:

1. Create an interface: ISingleton (containing whatever actions you want it do to)

2. And your type: Singleton : ISingleton

3. Assuming you have access to a UnityContainer:

IUnityContainer _singletonContainer = new UnityContainer(); // or whatever code to initialize the container

1. When you are ready to create your type use (assuming you are using Unity for DI):

_singletonContainer.RegisterType(typeof(ISingleton), new Singleton(params));

1. If you want to grab the singleton just use:

var localSingletonVar = _singletonContainer.Resolve<ISingleton>();

Note: If the container doesn't have a type registered for the ISingleton interface, then it should either throw exception, either return null.

Old Answer:

public class Singleton
{

    private static Singleton instance = null;

    private Singleton(String arg1, String arg2)
    {
    }

    public static Singleton getInstance(String arg1, String arg2)
    {
        if (instance != null)
        {
            throw new InvalidOperationException("Singleton already created - use getinstance()");
        }
        instance = new Singleton(arg1, arg2);
        return instance;
    }

    public static Singleton getInstance()
    {
        if (instance == null)
            throw new InvalidOperationException("Singleton not created - use GetInstance(arg1, arg2)");
        return instance;
    }
}

I'd go with something similar (you could need to check if instance was created too), or, if your DI container supports throwing exception on non registered types, i would go with that.

ATTN: Non thread safe code :)

Solution 4

The double locking singleton solution provided by annakata will not work everytime on all platforms. there is an flaw in this approach which is well documented. Do not use this approach or you will end up with problems.

The only way to solve this issue is to use the volatile keyword e.g.

private static volatile Singleton m_instance = null;

This is the only thread safe approach.

Solution 5

/// <summary> Generic singleton with double check pattern and with instance parameter </summary>
/// <typeparam name="T"></typeparam>
public class SingleObject<T> where T : class, new()
{
    /// <summary> Lock object </summary>
    private static readonly object _lockingObject = new object();

    /// <summary> Instance </summary>
    private static T _singleObject;

    /// <summary> Protected ctor </summary>
    protected SingleObject()
    {
    }

    /// <summary> Instance with parameter </summary>
    /// <param name="param">Parameters</param>
    /// <returns>Instance</returns>
    public static T Instance(params dynamic[] param)
    {
        if (_singleObject == null)
        {
            lock (_lockingObject)
            {
                if (_singleObject == null)
                {
                    _singleObject = (T)Activator.CreateInstance(typeof(T), param);
                }
            }
        }
        return _singleObject;
    }
}

View more solutions

Comments

[RPM1984]

Now, THAT is a singleton, not that i use them explicitly (i let DI do them, and only for things like logging). +1

[annakata]

I'm gonna go ahead and assume that whateva decided to downvote a functionally correct answer. Go team.

[RPM1984]

no, he didn't (look at his profile). anyway, all this talk of singleton's has made me sleepy, i'm off to bed. :)

[Massimiliano Peluso]

-1 I disagree the solution because I think you are suggesting to use singleton patter where is shouldn't be used. a singleton is a class which only allows a single instance of itself to be created, and usually gives simple access to that instance If the same instance should be accessed for all requests with the same parameter, the factory pattern is appropriate .In this solution you can call Create(.....) as many time you want breaking the immutability of the class.

[Erich Kitzmueller]

Massimiliano, in my solution you can call create exactly once. And I think my texting makes it clear enough that I'm not really fond with the idea of parametriced singletons either.

[veljkoz]

@Massimiliano Peluso - now that is a usefull observation - a factory... what I'm after is actually a mix of singleton/factory, which actually boils down to IoC... Anyway, I think I have my answer now. Thanks

[Massimiliano Peluso]

You can call create(...) as many time you want : you just throw an exception if it's called twice or more. In the SingletonPattern if and instance already exist you should just return it. In this context the Songleton pattern is not appropiate that's why you have to find a way to work it around(throwing exception)

[Erich Kitzmueller]

I aggree that "Create" is not the best possible name, I would have prefered "Initialize" to make the intention more clear, but I took the naming from the original question to avoid confusion. Calling "Create" many times does not break the immutability. I do not aggree that the Singleton pattern is inappropriate in any case; if (and only if) the creation parameters are constant during run time (e.g. are passed by command line parameters or read from a config file), such a setup might actually make some sense.

[Admin]

talk about fixing your code... Holy crap, this is buggy. Fixed it for you.

[annakata]

T1, T2 were not intended as generics just proxies for a real type.