Factory method for objects with multiple complex constructors

Question

I'm refactoring a legacy codebase.
I have 4 very similar objects that I decided would be a good target for becoming polymorphic, so I moved all the common code to a base class and added an interface.

The real problem arises when trying to create those objects. Every class has 3 different constructors with 5-10 parameters each, used in different cases based on the context.
I already know those classes have too much going on, and would actually be better split in different, smaller classes, each one dealing with one case, however this is a legacy codebase and I can't do so many changes to the existing code.

At the moment my code is something like this:

public class HandlerFactory
{
    public static IHandler CreateInstance(HandlerType param1, string param2, ..., string param8)
    {
        switch(param1)
        {
            case HandlerType.Type1
                return new Handler1(param2, ..., param8)
            case HandlerType.Type2
                return new Handler2(param2, ..., param8)
            case HandlerType.Type3
                return new Handler3(param2, ..., param8)
            case HandlerType.Type4
                return new Handler4(param2, ..., param8)
        }
    }

    public static IHandler CreateInstance(HandlerType param1, int param2, ..., DateTime param10)
    {
        switch(param1)
        {
            case HandlerType.Type1
                return new Handler1(param2, ..., param10)
            case HandlerType.Type2
                return new Handler2(param2, ..., param10)
            case HandlerType.Type3
                return new Handler3(param2, ..., param10)
            case HandlerType.Type4
                return new Handler4(param2, ..., param10)
        }
    }

    public static IHandler CreateInstance(HandlerType param1, string param2, ..., CustomClass param5)
    {
        switch(param1)
        {
            case HandlerType.Type1
                return new Handler1(param2, ..., param5)
            case HandlerType.Type2
                return new Handler2(param2, ..., param5)
            case HandlerType.Type3
                return new Handler3(param2, ..., param5)
            case HandlerType.Type4
                return new Handler4(param2, ..., param5)
        }
    }
}

However, I hate having 3 factory methods, as I now have to edit three of them if I were to add another Handler (the main reason I'm doing this refactoring).

Is there a way I can keep just one factory method with a single switch?

Answer 1

The answer provided by @JohnWu is very elegant and I like it a lot. Allow me to provide an alternative.

You could use Abstract Factory Pattern. You would basically have one big factory of factories, which would then produce the concrete factories for concrete handlers. Then, should you need another handler, all you would need to do is write one factory for that particular handler. It would be something along these lines:

public interface IHandler
{
}

public class Handler1 : IHandler
{
    public Handler1(string param1, string param2)
    {
    }

    public Handler1(int param1, int param2)
    {
    }
}

public class Handler2 : IHandler
{
    public Handler2(string param1, string param2)
    {
    }

    public Handler2(int param1, int param2)
    {
    }
}

For simplicity, I have only two Handler classes with two different constructors. I believe the solution still can be applied to your case. Next, we have factories:

   public interface IHandlerFactory
    {
        IHandler Create(string param1, string param2);
        IHandler Create(int param1, int param2);
    }

    public class Handler1Factory : IHandlerFactory
    {
        public IHandler Create(int param1, int param2)
        {
            return new Handler1(param1, param2);
        }

        public IHandler Create(string param1, string param2)
        {
            return new Handler1(param1, param2);
        }
    }

    public class Handler2Factory : IHandlerFactory
    {
        public IHandler Create(int param1, int param2)
        {
            return new Handler2(param1, param2);
        }

        public IHandler Create(string param1, string param2)
        {
            return new Handler2(param1, param2);
        }
    }

    public class HandlerFactory
    {
        public static IHandler Create(IHandlerFactory factory, string param1, string param2)
        {
            return factory.Create(param1, param2);
        }

        public static IHandler Create(IHandlerFactory factory, int param1, int param2)
        {
            return factory.Create(param1, param2);
        }
    }

The alternative here would be to have a HandlerFactory class that would have a property of IHandlerFactory type, and the methods would not be static and would not have IHandlerFactory parameter in them. Obviously, an object of HandlerFactory class would need to be instantiated at some point.

The usage would go somehow like this:

public class Example
{
    public void ExampleMethod()
    {
        Handler1Factory factory1 = new Handler1Factory();
        var handler1 = HandlerFactory.Create(factory1, "parameter1", "parameter2");

        Handler2Factory factory2 = new Handler2Factory();
        var handler2 = HandlerFactory.Create(factory2, 1, 2);
    }
}

The solution clearly divides the code by purpose, and most importantly, should you have the need for another handler, you would not need to change the existing code. All you would have to do is write a factory for that particular handler.

Remember, this is only possible because all handlers have the constructors with same parameter lists. The design would be somewhat different if that was not the case.

Answer 2

This solution eliminates the case/switch and replaces it with a generic argument. Normally you can't create objects this way unless you use the new constraint, which doesn't allow any constructor arguments; I stole this workaround from another answer.

Also, there is only one factory method here, and constructor arguments are accepted as a variable-sized list using the params keyword.

public class HandlerFactory
{
    public static T CreateInstance<T>(params object[] constructorArguments) where T : class, IHandler
    {
        return (T)Activator.CreateInstance(typeof(T), constructorArguments);
    }
}

As you can see, this method is much shorter, has no case/switch, and only requires one prototype. And the nice thing is if you ever add Handler4 or any other subclass, you don't need to touch the factory. Just make sure that Handler4 implements IHandler and you're already all set.

You can call it like this:

var o = HandlerFactory.CreateInstance<Handler3>(param1, param2);

Because the return type is T you have your choice of declaring o as a var (and the compiler will infer the concrete type) or you could use

IHandler o = HandlerFactory.CreateInstance<Handler3>(param1, param2);

if you prefer to only work with the interface.

Answer 3

As you said, the existing code has design problems as it is. First, having methods and constructors with so many (different!) parameters is a design smell.

If you don't fix the root cause of the complexity, you can make life easier by adding another layer of abstraction. This will ease maintaining the code base until you get time to refactor it (or move to a new one :) ).

1. Make classes for the passed parameters (Param1, Param2, Param3).
2. [Hack] Add a common interface (say IParameter) for the 3 classes.
   • It's a hack because the interface is not really common, but if you want to have only one switch in your Factory, that's a solution (the one I could find).
3. Create a factory for the IParameter interface. The factory itself decides what to build based on the parameters it is set.
4. [Hack] In the constructors of the existing classes, add a switch statement based on the concrete class type of the IParameter object.
   • It's a hack because this is not what interfaces should be used for. Interfaces should provide an abstraction for common behavior.

DonNet Fiddle with working example: https://dotnetfiddle.net/NoChOk

The code for perusal:

using System;

public class Program
{
    public void Main()
    {
        IHandler first = HandlerFactory.CreateInstance(
            HandlerType.Handler1,
            ParamBuilder.start().withOne("x").withTwo("y").build());
        IHandler second = HandlerFactory.CreateInstance(
            HandlerType.Handler2,
            ParamBuilder.start().withAlpha(1).withBeta(new DateTime()).build());
        IHandler third = HandlerFactory.CreateInstance(
            HandlerType.Handler3,
            ParamBuilder.start().withMickey(":)").withMouse(1L).build());

        first.DoStuff();
        second.DoStuff();
        third.DoStuff();
    }

    class HandlerFactory
    {
        public static IHandler CreateInstance(HandlerType type, IParam param)
        {
            switch (type) {
            case HandlerType.Handler1: return new Handler1(param);
            case HandlerType.Handler2: return new Handler2(param);
            case HandlerType.Handler3: return new Handler3(param);
            default: throw new NotSupportedException();
            }
        }
    }

    enum HandlerType { Handler1, Handler2, Handler3 
    }

    interface IHandler { void DoStuff(); }

    class Handler1 : IHandler {
        private Param1 numbers; private Param2 letters; private Param3 disney;

        public Handler1(IParam param) {
            if (param is Param1) numbers = (Param1) param;
            if (param is Param2) letters = (Param2) param;
            if (param is Param3) disney = (Param3) param;
        }

        public void DoStuff() { Console.WriteLine("I am alive - Handler 1");}
    }

    class Handler2 : IHandler {
        private Param1 numbers; private Param2 letters; private Param3 disney;

        public Handler2(IParam param) {
            if (param is Param1) numbers = (Param1) param;
            if (param is Param2) letters = (Param2) param;
            if (param is Param3) disney = (Param3) param;
        }

        public void DoStuff() { Console.WriteLine("I am alive - Handler 2");}
    }

    class Handler3 : IHandler {
        private Param1 numbers;
        private Param2 letters;
        private Param3 disney;

        public Handler3(IParam param) {
            if (param is Param1) numbers = (Param1) param;
            if (param is Param2) letters = (Param2) param;
            if (param is Param3) disney = (Param3) param;
        }

        public void DoStuff() { Console.WriteLine("I am alive - Handler 3");}
    }

    interface IParam {
        string One { get; } string Two { get; } // first param class

        int Alpha { get; } DateTime Beta { get; } // second param class

        string Mickey { get; } long Mouse { get; } // third param class
    }

    class ParamBuilder {
        private string one, two;
        private int? alpha; private DateTime? beta;
        private  string mickey; private long? mouse;

        public static ParamBuilder start() { return new ParamBuilder(); }

        public IParam build() {
            if (!string.IsNullOrEmpty(one) && !string.IsNullOrEmpty(one)) {
                return new Param1(one, two);
            }
            if (alpha.HasValue && beta.HasValue) {
                return new Param2(alpha.Value, beta.Value);
            }
            if (!string.IsNullOrEmpty(mickey) && mouse.HasValue) {
                return new Param3(mickey, mouse.Value);
            }

            throw new NotSupportedException();
        }

        public ParamBuilder withOne(string one) { this.one = one; return this; }
        public ParamBuilder withTwo(string two) { this.two = two; return this; }
        public ParamBuilder withAlpha(int alpha) { this.alpha = alpha; return this; }
        public ParamBuilder withBeta(DateTime beta) { this.beta = beta; return this; }
        public ParamBuilder withMickey(string m) { this.mickey = m; return this; }
        public ParamBuilder withMouse(long m) { this.mouse = m; return this; }
    }

    class Param1 : IParam {
        public string One { private set; get; }
        public string Two { private set; get; }

        public int Alpha { get {throw new NotImplementedException();} }
        public DateTime Beta { get {throw new NotImplementedException();} }
        public string Mickey { get {throw new NotImplementedException();} }
        public long Mouse { get {throw new NotImplementedException();} }

        public Param1(string one, string two) {
            One = one; Two = two;
        }
    }

    class Param2 : IParam {
        public int Alpha { private set; get; }
        public DateTime Beta { private set; get; }

        public string One { get {throw new NotImplementedException();} }
        public string Two { get {throw new NotImplementedException();} }
        public string Mickey { get {throw new NotImplementedException();} }
        public long Mouse { get {throw new NotImplementedException();} }

        public Param2(int alpha, DateTime beta) {
            Alpha = alpha; Beta = beta;
        }
    }

    class Param3 : IParam {
        public string Mickey { private set; get; }
        public long Mouse { private set; get; }

        public string One { get {throw new NotImplementedException();} }
        public string Two { get {throw new NotImplementedException();} }
        public int Alpha { get {throw new NotImplementedException();} }
        public DateTime Beta { get {throw new NotImplementedException();} }

        public Param3(string mi, long mo) {
            Mickey = mi; Mouse = mo;
        }
    }
}