Dependency Injection and Factory resolution

Question

In the below example, assume FooA and FooB each have constructors that have a large amount of dependencies being injected into them.

If I have a class that needs to determine which IFoo implementation to use, what is my best method to resolve that? I currently have two possible versions, but I'm not sure either is correct.

public interface IFactory
{
    IFoo CreateFoo(bool makeA);
}

public class FactoryA : IFactory
{
    private readonly IServiceProvider _services;

    public FactoryA(IServiceProvider services)
    {
        _services = services;
    }
    public IFoo CreateFoo(bool makeA)
    {
        if (makeA)
        {
            return _services.GetRequiredService<FooA>();
        }
        else
        {
            return _services.GetRequiredService<FooB>();
        }
    }
}

public class FactoryB : IFactory
{
    private readonly FooA _fooA;
    private readonly FooB _fooB;

    public FactoryB(FooA fooA, FooB fooB)
    {
        _fooA = fooA;
        _fooB = fooB;
    }
    public IFoo CreateFoo(bool makeA)
    {
        if (makeA)
        {
            return _fooA;
        }
        else
        {
            return _fooB;
        }
    }
}

I feel that FactoryA is a more correct process, but others at work disagree with passing in IServiceProvider, however they don't currently have a better recommendation. FactoryB was the methodology before. My main reason for changing it was complications with testing. I like the clean nature of A vs B and it greatly simplifies unit testing.

In practice an Enum was being passed in and there was a small switch statement vs the bool and if/else that is currently demo'd.

var factory = serviceProvider.GetRequiredService<IFactory>();
var foo = factory.CreateFoo(false);

Answer 1

The first thing I would note, which might be a little nitpicky, is the use of the term factory. To me a factory implies the objects are being created, whereas FactoryB is handing out already created objects, and I suspect FactoryA does too.

Your coworkers probably don't like FactoryA because it looks like a Service Locator, which is considered by many as an anti-pattern. Mainly it comes down to the fact that users of FactoryA can't tell what dependencies it really needs. Sure, it needs an IServiceProvider. But what services need to be registered with that provider?

Now, if FactoryA class were part of your Composition Root, then it could be acceptable. The Composition Root is responsible for composing all the other objects in your application. It's already making decisions about what classes are needed, and it's the place that would have registered classes with the service provider anyway. Plus, it would never be reused, so you don't have the problem of users not knowing what dependencies FactoryA requires. It's usually used for creating objects, though, not providing existing ones, so if IServiceProvider assumes the objects already exists, this may not apply here. This page describes why this wouldn't be considered using Service Locator anti-pattern.

Having said all that, this probably could be simpler than you currently have it. If the objects are already created somewhere else (as FactoryB implies), and you just need to choose one based on the value of an enum, why not just put them in a Dictionary<SomeEnum, IFoo>?