Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I did this as an exercise just to practice/improve using generics.

Independent of how useful this implementation of a Singleton is, how is my coding in terms of using generics and any other aspect of class design of code style?

void Main()
{
    var a = Singleton<MyClass>.Value;
    var b = Singleton<MyClass, MyClassFactory>.Value;
    var c = Singleton<MyClass>.Value;
    var d = Singleton<MyClass, MyClassFactory>.Value;
    var e = Singleton<MyOtherClass>.Value;
    var f = Singleton<MyOtherClass>.Value;
    var g = Singleton<MyOtherClass, MyOtherFactory>.Value;
    var h = Singleton<MyOtherClass, MyOtherFactory>.Value;
}

class SingletonBase
{
    protected static object Locker = new LockerObject();
}

class Singleton<T> : SingletonBase where T : new() 
{
    static T StaticT;

    public static T Value 
    {
        get
        {
            lock (Locker)
            {
                if(StaticT == null)
                {
                    StaticT = Activator.CreateInstance<Factory<T>>().Create();
                }
                else
                {
                    Console.WriteLine ("Singleton<T>::Value" + typeof(T).Name + " is already created");
                }
            }
            return StaticT;
        }
    }
}

class Singleton<T, F> : SingletonBase where T : new() where F : IFactory<T>, new()
{
    static T StaticT;

    public static T Value 
    {
        get
        {
            lock (Locker)
            {
                if(StaticT == null)
                {
                    StaticT = new F().Create();
                }
                else
                {
                    Console.WriteLine ("Singleton<T, F>::Value" + typeof(T).Name + " is already created");
                }
            }
            return StaticT;
        }
    }
}

class LockerObject
{
    Guid myGUID;
    public LockerObject()
    {
        this.myGUID = Guid.NewGuid();
        Console.WriteLine ("New LockerObject " + this.myGUID.ToString());
    }
}

interface IFactory<T>
{
    T Create();
}

class Factory<T> : IFactory<T> where T : new()
{
    public T Create()
    {
        Console.WriteLine ("Factory<T>::Create()");  
        return new T();
    }
}

class MyClassFactory : IFactory<MyClass>
{
    public MyClass Create()
    {
        Console.WriteLine ("MyClassFactory::Create()");    
        return new MyClass();
    }
}

class MyClass
{
    public MyClass()
    {
        Console.WriteLine ("MyClass created");
    }
}


class MyOtherClass
{
    public MyOtherClass()
    {
        Console.WriteLine ("MyOtherClass created");
    }
}

class MyOtherFactory : IFactory<MyOtherClass>
{
    public MyOtherClass Create()
    {
        Console.WriteLine ("MyOtherFactory::Create()");    
        return new MyOtherClass();
    }
}

Output:

New LockerObject 36aa2282-d745-43ca-84d2-998a78e39d51
Factory<T>::Create()
MyClass created
MyClassFactory::Create()
MyClass created
Singleton<T>::ValueMyClass is already created
Singleton<T, F>::ValueMyClass is already created
share|improve this question
2  
a little offtopic: those are not singletons. As long as class T is required to have a public parameterless constructor, anyone can create a new instance of it. Same problem with the version using IFactory<T>. –  w0lf May 23 '12 at 7:47
    
understood, good point. I was only designing it for code that makes the assumption that a DI container is able to control all object instantiation... –  Aaron Anodide May 23 '12 at 15:34
    
Does this design assure in multithreading case it creates only one object of this class? –  Prasad S Deshpande Oct 17 '12 at 4:45
    
Have a read of csharpindepth.com/Articles/General/Singleton.aspx –  dreza Jul 7 '13 at 20:33

1 Answer 1

About your locking strategy to create the singleton, I would use a double check to avoid too much locking:

if(x)
{
    lock(Locker)
        if(x) //again once we got the lock
            DoStuff();
} 
else 
{
    FooBar();
}

Like this, you will hopefully use the lock only once when you first call the instance.

share|improve this answer
1  
+1 for more details go through ibm.com/developerworks/java/library/j-dcl/index.html –  Prasad S Deshpande Oct 17 '12 at 4:56
    
@PrasadSDeshpande I'm not sure your link is relevant. It shows that the double-locking behavior shouldn't be used with Java, but this question is about C#. –  Zonko Oct 19 '12 at 14:47
    
Double checked locking has visibility problems in Java. I guess @Zonko thought that the same could be true for C# too. stackoverflow.com/questions/394898/… –  palacsint Jul 7 '13 at 9:17
1  
I would stay away from this method as suggested by c# in Depth (Jon Skeet) - csharpindepth.com/Articles/General/Singleton.aspx –  dreza Jul 7 '13 at 20:32

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.