How to avoid switch case in factory

Question

I have this factory class which converts a Foo into a list of Bar objects. Foo is a very complex object which I flatten into a list of simple Bar objects. There are about 60 different bits of data that could be transformed from a Foo into a Bar. The following implementation works but there's definite scope for improvement here.

public class FooToBarsConverter
{
    public List<Bar> Convert(Foo foo)
    {
        return Enum.GetValues(typeof(BarTypeEnum))
            .Cast<BarTypeEnum>()
            .Select(barType => CreateBar(foo, barType))
            .Where(newBar => newBar != null)
            .ToList();
    }


    public Bar CreateBar(Foo foo, BarTypeEnum barType)
    {
        switch (barType)
        {
            case BarTypeEnum.TypeA:
                return CreateTypeA(foo);

            case BarTypeEnum.TypeB:
                return CreateTypeB(foo);
        }

        return null;
    }

    private Bar CreateTypeA(Foo foo)
    {
        return new Bar(...);
    }

    private Bar CreateTypeB(Foo foo)
    {
        return new Bar(...);
    }
}

Ideally I'd like to avoid having to write a new case to the switch every time a new BarType is added. Perhaps a dictionary of types and delegate functions but that would still require a mapping of sorts? Is there any feature of the language that I can exploit to avoid this switch case a make the compiler choose the create create function?

---

Assuming you don't mind the factory methods being statics this does neaten it up a bit without needing the cruft of having to create ~60 more sub-classes to get the type system to do the work for me. I think the statics aren't needed if you make it a func with the factory as well but I've not got that far yet. The statics don't particularly bother me with it just being data transposition

private static readonly IDictionary<BarTypeEnum, Func<Foo, Bar>>
  CreateLookup = new Dictionary<BarTypeEnum, Func<Foo, Bar>>
   {
       { BarTypeEnum.TypeA, CreateTypeA },
       { BarTypeEnum.TypeB, CreateTypeB }
   };

 public Bar Create(Foo foo, BarTypeEnum barType)
 {
     Func<Foo, Bar> createDelegate;
     CreateLookup.TryGetValue(barType, out createDelegate);
     return createDelegate != null ? createDelegate(foo) : null;
 }

 private static Bar CreateTypeA(Foo foo) { ... }
 private static Bar CreateTypeB(Foo foo) { ... }

Answer 1

Personally I don't mind a switch in a factory method, it's readable, it's neat and doesn't sarcrafice the end goal of what a factory method is for - keeping the initialization code together.

However, that being said, I wonder if a custom attribute could tidy this up a bit for you. Going on the assumption all the CreateBarX methods create an instance of Bar initializing the specific properties from Foo.

[System.AttributeUsage(System.AttributeTargets.Field)]
public class FooConverter : System.Attribute
{
    public string Parameters;

    public Bar GetInstance(Foo foo)
    {
        var propNames = String.IsNullOrEmpty(Parameters) ? new string[] { } : Parameters.Split(',').Select(x => x.Trim());
        var parameters = foo.GetType().GetProperties().Where(x => propNames.Contains(x.Name)).Select(x => x.GetValue(foo));
        return (Bar)Activator.CreateInstance(typeof(Bar), parameters.ToArray());
    }
}

// extension helpers
public static class EnumExt
{
    public static Bar GetInstance(this BarTypeEnum value, Foo foo)
    {
        var converterAttr = value.GetAttribute<FooConverter>();
        return converterAttr != null ? converterAttr.GetInstance(foo) : null;
    }

    public static T GetAttribute<T>(this System.Enum value)
    {
        FieldInfo fi = value.GetType().GetField(value.ToString());
        var attributes = fi.GetCustomAttributes(typeof(T), false);
        return attributes.Length > 0 ? (T)attributes[0] : default(T);
    }   
}

Which would allow you to do

public enum BarTypeEnum
{
    [FooConverter] // no properties mapped
    TypeA,
    [FooConverter(Parameters="Prop1")] // map Prop1 from Foo to Bar
    TypeB,
    TypeC, // no instance
    [FooConverter(Parameters="Prop1, Prop2")] // map Prop1/2 from Foo to Bar
    TypeD, 
    TypeE // no instance
}

public List<Bar> Convert(Foo foo)
{
    return Enum.GetValues(typeof(BarTypeEnum))
        .Cast<BarTypeEnum>()
        .Select(barType => barType.GetInstance(foo))
        .Where(newBar => newBar != null)
        .ToList();
}

And that's all you need!

However, there are some limitations to this approach with respect to the parameter injection, CreateInstance will only match the constructor based on a signature which matches the data type i.e.

 // this will call Bar(string prop1, string prop2)
 Activator.CreateInstance(typeof(Bar), new object[] { "Property1", "Property2" });

 // where as this will car Bar(string prop1)
 Activator.CreateInstance(typeof(Bar), new object[] { "Property2" });

The ordering is important as well

 // this will call Bar(string prop1, string prop2) so Prop1 = "Property2"
 Activator.CreateInstance(typeof(Bar), new object[] { "Property2", "Property1" });

However, there are ways around this - for the most part this will probably work well.

Answer 2

I hate 'case's and I like Generics therefore I slightly changed interface of FooToBarsConverter:

public interface IFooToBarsConverter
{
    List<Bar> Convert(Foo foo);
    Bar CreateBar<TBarType>(Foo foo) where TBarType : Bar;
}

There is an implementation:

public class FooToBarsConverter : IFooToBarsConverter
{
    public List<Bar> Convert(Foo foo)
    {
        return new List<Type>
        {
            typeof(Bar.BarA),
            typeof(Bar.BarB)
        }.Select(it => CreateBar(foo, it))
        .ToList();
    }

    public Bar CreateBar<T>(Foo foo)
        where T : Bar
    {
        return CreateBar(foo, typeof(T));
    }

    private Bar CreateBar(Foo foo, Type barType)
    {
        return typeof(Bar).IsAssignableFrom(barType)
            ? (Bar)Activator.CreateInstance(barType, foo)
            : null;
    }
}

public class Foo
{
}

public abstract class Bar
{
    private Bar(Foo foo)
    {

    }

    public class BarA : Bar
    {
        public BarA(Foo foo)
            : base(foo)
        {
        }
    }

    public class BarB : Bar
    {
        public BarB(Foo foo)
            : base(foo)
        {
        }
    }
}

... and a test that tests it:

    [TestMethod]
    public void TestMethod()
    {
        // arrange
        var foo = new Foo();
        var target = new FooToBarsConverter();

        // act + assert
        var list = target.Convert(foo);
        list.Should().HaveCount(2);
        list.Should().NotContainNulls();

        var typeA = target.CreateBar<Bar.BarA>(foo);
        typeA.Should().BeAssignableTo<Bar.BarA>();

        var typeB = target.CreateBar<Bar.BarB>(foo);
        typeB.Should().BeAssignableTo<Bar.BarB>();
    }

Answer 3

Is there any feature of the language that I can exploit to avoid this switch case a make the compiler choose the create create function?

Yes. It's called polymorphism

Check this video: Jimmy Bogard - Crafting Wicked Domain Models on how an enum could be converted into a polimorhic class hierachy.

Basically you create an abstract class called BarTypeEnum that feels like an enum and create n derived types, one for each enum value. Then you could have this method

public abstract Bar CreateBar(Foo foo);

and override it in every subclass each returning a diferent subtype of Bar

e.g.

public override Bar CreateBar(Foo foo)
{
    return CreateTypeA(foo);
}

BTW: The enumeration class he talks about is on NuGet as the NuGet package Enumeration

EDIT I just checked and the nuget package class is not the same as the video. It is a Generic, nonpolimorphic way to implement it though

Answer 4

Not a huge fan of this because it's a bit hard to read, but you can define a custom attribute, mapping each enum value to its method. You'd use reflection to find and execute the appropriate method.

public class BarChooserAttribute : Attribute
{
    public BarChooserAttribute(BarTypeEnum barType) { BarType = barType; }
    public BarTypeEnum BarType { get; set; }
}

public static class CreateBarMethods
{
    [BarChooser(BarTypeEnum.TypeA)]
    public static Bar CreateTypeA(Foo foo)
    {
        return new Bar { Message = "A" };
    }

    [BarChooser(BarTypeEnum.TypeB)]
    public static Bar CreateTypeB(Foo foo)
    {
        return new Bar { Message = "B" };
    }
}

public static Bar CreateBar(Foo foo, BarTypeEnum barType)
{
    var methodWrapper = typeof(CreateBarMethods).GetMethods(BindingFlags.Public | BindingFlags.Static)
        .Select(m => new { Method = m, Att = (BarChooserAttribute)m.GetCustomAttributes(typeof(BarChooserAttribute), false).Single() })
        .Single(x => x.Att.BarType == barType);
    return (Bar)methodWrapper.Method.Invoke(null, new[] { foo });
}

To improve performance, you can map the methods into a dictionary one time and retrieve them from the dictionary each time. Additionally, you can use expression trees to compile the methods into lambda expressions, so you only have to do reflection once instead of each time you make the call. Significant performance improvements, to get significantly harder-to-read code, so it's a trade-off.