How can I avoid code duplication of a static method in multiple classes

user_7330525 0
Viewed 366

I have multiple classes containing duplicated code, especially members and most important a static method that will create a new instance of the class and returning this instance: either a previously created instance registered in a dictionary or a new instance by calling the constructor.

An interface is no option, because I have the static method. I tried to solve the problem by introducing a base class that implements this static method, but I can not find a way to create and return a spefific child class properly.

Below is a code example of the current situation with class A and class B showing duplicated code.

public class A
{
    private static readonly IDictionary<string, A> Registry = new Dictionary<string, A>();
    public string Name { get; set; }

    public A(string name)
    {
        this.Name = name;
    }

    public static A GetA(string instanceName)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = new A(instanceName);
            }
            return newInstance;
        }
    }
}

And then in class B again there is a member Name and the GetX() Method.

public class B
{
    private static readonly IDictionary<string, B> Registry = new Dictionary<string, B>();
    public string Name { get; set; }

    public B(string name)
    {
        this.Name = name;
    }

    public static B GetB(string instanceName)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = new B(instanceName);
            }
            return newInstance;
        }
    }
}

Is there a possibility to avoid this kind of code duplication by introducing a base class or any other way?

static-polymorphism code-duplication inheritance c#
4 Answers

This might be a little cleaner:

public class B: RegistryInstance<B>
{
    public string Name { get; set; }

    public B(string name)
    {
        this.Name = name;
    }
}

public class A : RegistryInstance<A>
{
    public string Name { get; set; }

    public A(string name)
    {
        this.Name = name;
    }
}

public abstract class RegistryInstance<T> where T:class
{
    protected static readonly IDictionary<string, T> Registry = new Dictionary<string, T>();

    public static T GetInstance(string instanceName)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = (T)Activator.CreateInstance(typeof(T), new object[] { instanceName });
                Registry.Add(instanceName, newInstance);
            }
            return newInstance;
        }
    }
}
user_4607820

Are you looking for a generic base class?

public abstract class BaseRegistryGetter<T>
{
    private static readonly IDictionary<string, T> Registry = new Dictionary<string, T>();
    public string Name { get; set; }

    public BaseRegistryGetter(string name)
    {
        this.Name = name;
    }

    public static T GetValue (string instanceName, Func<string, T> creator) {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = creator(instanceName);
            }
            return newInstance;
        }
    }
}

And then use it like this:

public class A : BaseRegistryGetter<A>
{
    public A(string name) : base(name)
    {
    }

    public static A GetA(string instanceName)
    {
        return BaseRegistryGetter<A>.GetValue(instanceName, s => new A(s));
    }
}

The source for the awkward approach to make sure there is a string-constructor for A can be found here.

user_30859850

I think this should work. You can adapt it to fit your needs. Also, there was a bug in your code: you forgot to add to the Registry when you were creating a new instance.

class Program
{
    static void Main(string[] args)
    {
        A a1 = A.GetInstance("a");
        A a2 = A.GetInstance("aa");
        A a3 = A.GetInstance("a");

        B b1 = B.GetInstance("a");
        B b2 = B.GetInstance("aa");
        B b3 = B.GetInstance("a");

        Console.WriteLine(a1 == a2); //false
        Console.WriteLine(a1 == a3); //true

        Console.WriteLine(b1 == b2); //false
        Console.WriteLine(b1 == b3); //true

        Console.ReadKey();
    }
}

public class A : Generic<A>
{
    public A(string name)
        : base(name)
    {
    }
}

public class B : Generic<B>
{
    public B(string name)
        : base(name)
    {
    }
}

public abstract class Generic<T> where T : Generic<T>
{
    private static readonly IDictionary<string, T> Registry = new Dictionary<string, T>();
    public string Name { get; set; }

    public Generic(string name)
    {
        this.Name = name;
    }

    public static T GetInstance(string instanceName)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = (T)Activator.CreateInstance(typeof(T), instanceName);
                Registry.Add(instanceName, newInstance);
            }
            return newInstance;
        }
    }
}
user_10841680

All the other answers try to solve this with generics, but it might be the case you wouldn't want to do this. First, it could be an unnecessary restriction further along that could end up causing variance issues. Second, it only solves one level of inheritance, if there is more, you are stuck again with the same problem:

 class Base<T> { ... }
 class A: Base<A> { ... }
 class B: A { //How does the generic base class help? }

There are general solutions without the use generics that entails just a little code duplication. One could be the following:

public class Base
{
    static readonly IDictionary<string, Base> Registry = 
        new Dictionary<string, Base>();

    protected static Base GetBase(string instanceName,
                                  Func<Base> creator)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = creator();
            }   

            return newInstance;
        }
    }

    //...
}

And now yor derived types can impement a strongly typed delegated method:

public class A: Base
{
    public A(string instanceName)
        :base(instanceName)
    {
    }
    public static A GetA(string instanceName)
        => GetBase(instanceName, () => new A(instanceName)) as A;
}

public class B: Base
{
    public B(string instanceName)
        :base(instanceName)
    {
    }
    public static B GetB(string instanceName)
        => GetBase(instanceName, () => new B(instanceName)) as B;
}
user_7678900
Related