C# – Factory that returns multiple implementations of the same interface

cdependency-injectionfactoryrefactoring

A few developers and I are attempting to refactor a class that has grown too large. Currently this class is around 3K lines long. The goal of the refactor is to make the logic more maintainable.

The class holds our validation logic, which performs various checks to make sure things are good before moving to the next step of the process.

Currently each validation is a function of the validation class. They all have the same method signature.

One idea we had is to create a class for each validation, with a factory to give us a list of all validations that are needing to run.

A simple implementation of this is below:

Here's the validation interface:

public interface IValidationType
{
    bool Validate();
}

With an implementation looking like this:

public class ValidationType1 : IValidationType
{
    private readonly IDependency1 _dependency1;

    public ValidationType1(IDependency1 dependency1)
    {
        _dependency1 = dependency1;
    }

    public bool Validate()
    {
        return _dependency1.SomeFunction();
    }
}

With a factory class that accepts IValidationType's as dependencies:

public class ValidationFactory : IValidationFactory
{
    private readonly IList<IValidationType> _validations;

    public ValidationFactory(
        IValidationType validation1,
        IValidationType validation2)
    {
        _validations = new List<IValidationType>
        {
            validation1,
            validation2
        };
    }

    public IEnumerable<IValidationType> RetrieveValidations()
    {
        return _validations;
    }
}

Currently our plan is to abuse our IOC container to handle the dirty work with named registers:

public class ValidationAutofacModule : Module
{
    protected override void Load(ContainerBuilder builder)
    {
        builder.RegisterType<ValidationType1>().Named<IValidationType>("Validation1");
        builder.RegisterType<ValidationType2>().Named<IValidationType>("Validation2");

        builder.Register<IValidationFactory>(
            x => new ValidationFactory(
                x.ResolveNamed<IValidationType>("Validation1"),
                x.ResolveNamed<IValidationType>("Validation2")));
    }
}

One of our concerns with this implementation is that for each new validation created the developer would need to register it in the factory, and also in the IOC container. It also smells having to abuse the IOC container to get this to work.

Is this an anti-pattern? Is there a better way?

Best Answer

Asking for a collection of all implementations of one interface can be an effective pattern.

When your interface has a natural composite implementation, this is a good way to allow extension points. I think of this as a "plugin" approach: anyone can write a new implementation of your interface and its behavior gets naturally mixed in with all the other implementations.

A "natural composite" is a valid implementation of your interface that delegates to its component implementations and combines their results in a way that is natural to its purpose. Your case is straightforward: to implement Validate in the composite, just call Validate on all components and combine the results with and.

Named dependencies and factories are not the right way to handle this.

Yes, you are abusing your IOC container. The principle of DI is to ask for what you need. You need an IEnumerable<IValidationType> (an IList is inappropriate here because you should not be modifying it and don't need random access). But instead of just asking for an IEnumerable<IValidationType>, you're asking for two different implementations. As you identified, this violates open/closed because your factory will change every time you come up with a new implementation.

I also don't understand the point of your factory. It seems to have 0 responsibilities. Instead of the factory, write the composite.

public class CompositeValidationType : IValidationType
{
    public CompositeValidationType(IEnumerable<IValidationType> components) { ... }
    public bool Validate() { return components.All(o => o.Validate()); }
}

Then get your IOC container to register this composite with the right component implementations as the canonical IValidationType. Reflection can be your friend here.

I am vehemently against ever using named dependencies; it breaks the sensible rule that renaming a method parameter should always be a safe refactor.