Question

I have a factory class that creates a couple of different types of class. The factory is registered with the container. What is the recommended way of creating the classes inside the factory, given that they also have dependencies. I clearly want to avoid a dependency on the container but if I new those classes then they won't be using the container. e.g.

public class MyFactory
{
    public IMyWorker CreateInstance(WorkerType workerType)
    {
        if (workerType == WorkerType.A)
              return new WorkerA(dependency1, dependency2);
        
        return new WorkerB(dependency1);    
    }
}

So the question is where do I get those dependencies from.

One option could be to make them dependencies of the factory. e.g.

public class MyFactory
{
    private Dependency1 dependency1;
    private Dependency2 dependency2;

    public MyFactory(Dependency1 dependency1, Dependency2, dependency2)
    {
        this.dependency1 = dependency1; this.dependency2 = dependency2;
    }

    public IMyWorker CreateInstance(WorkerType workerType)
    {
        if (workerType == WorkerType.A)
              return new WorkerA(dependency1, dependency2);
        
        return new WorkerB(dependency1);

    }
}

Another could be to register the worker types and make those dependencies of the factory e.g.

public class MyFactory
{
    private IWorkerA workerA;
    private IWorkerB workerB;

    public MyFactory(IWorkerA workerA, IWorkerB, workerB)
    {
        this.workerA = workerA; this.workerB = workerB;
    }

    public IMyWorker CreateInstance(WorkerType workerType)
    {
        if (workerType == WorkerType.A)
              return workerA;
        
        return workerB;  
    }
}

With the first option I feel like I am leeching the dependencies of the workers into the factory. With the second option the workers are created when the factory is created.

Était-ce utile?

La solution 2

This is something of a classic question.

Both of your solutions could be problematic if the number of different implementations increases, especially if the dependencies for each of the implementations have a lot of variance. You could end up with a constructor that takes 20 parameters.

My preferred implementation is to just have the factory class reference the container, and resolve the needed instance that way.

Some might argue that this is no better than the Service Locator anti-pattern, but I don't feel like there is a perfect solution to this problem, and doing it this way seems the most natural to me.

Autres conseils

I agree with @Phil, letting the factory take a dependency on the container is fine, but there's one peace of information missing in his answer.

You are probably trying to prevent taking a dependency on the container because you try to stay away from the Service Locator anti-pattern. I agree that Service Locator is an anti-pattern and should be prevented.

Whether or not a dependency on the container is an implementation of the Service Locator anti-pattern depends on where this consumer is defined. Mark Seemann explains this here:

A DI container encapsulated in a Composition Root is not a Service Locator - it's an infrastructure component.

So letting your factory depend on the container is fine as long as you define this MyFactory implementation inside your composition root.

When you do this you'll soon get into trouble since a class that is defined in the composition root can't be referenced from the rest of the application. But that problem is easily solved by defining an IMyFactory interface in the application and letting your factory implementation implement that interface (as you should do anyway to adhere to the Dependency Inversion Principle).

So your registration would become something like this:

container.RegisterSingleton<IMyFactory, MyFactory>();

And the implementation like this:

private sealed class MyFactory : IMyFactory
{
    private readonly Container container;

    public MyFactory(Container container)
    {
        this.container = container;
    }

    public IMyWorker CreateInstance(WorkerType workerType)
    {
        if (workerType == WorkerType.A)
              return this.container.GetInstance<IWorkerA>();

        return this.container.GetInstance<IWorkerB>();
    }
}

The solution depends highly on the lifetime of the two dependencies in my opinion. Can these dependencies be shared across objects, then You can register them in the container and pass them to the factory and reuse them. If every "product" of the factory should have it's own instance, then maybe You could consider building a separate factory for those dependencies (of course if they belong to the same “family” of objects) and pass it to Your factory, and ask for an instance whenever You are creating an instance of IMyWorker. Alternatively You could consider using a Builder instead of a Factory for creating each dependency before creating the end product – IMyWorker in Your case.

Passing the container is considered a code smell unless you are implementing the composition root like for instance in WCF.

If You end up with a factory taking many dependencies in the constructor then you should see it as a hint, that something is wrong - most likely something is violating the Single Responsibility Principle – it simply knows too much ;)

A very good book talking about Dependency Injection is the book “Dependency Injection in .NET” by Mark Seemann which I recommend :)

While this question is subjective (the answer will be as well), I would say that your first approach is the appropriate one.

When you utilize Dependency Injection you have to understand what is an actual dependency. In this case, WorkerA and WorkerB are not really dependencies, but clearly Dependency1 and Dependency2 are dependencies. In a real-world scenario, I have used this pattern in my Micrsoft Prism applications.


Hopefully an example of my application will give you a better understanding of the pattern to use. I utilized the ILoggerFacade dependency. I had some view-models that resided in a separate assembly (the factory resided in that assembly as well). My individual IPlayerViewModels were not dependencies (which is why I didn't go the second route).

ShellViewModel.cs:

[Export]
public sealed class ShellViewModel : NotificationObject
{
    public ShellViewModel()
    {
         Players = new ObservableCollection<IPlayerViewModel>();

         // Get the list of player models
         // from the database (ICollection<IPlayer>)
         var players = GetCollectionOfPlayerModels();

         foreach (var player in players)
         {
             var vm = PlayerViewModelFactory.Create(player);

             Players.Add(vm);
         }
    }

    [Import]
    private IPlayerViewModelFactory PlayerViewModelFactory { get; set; }

    public ObservableCollection<IPlayerViewModel> Players { get; private set; }
}

IPlayerViewModelFactory.cs

public interface IPlayerViewModelFactory
{
    IPlayerViewModel Create(IPlayer player);
}

IPlayer.cs

public interface IPlayer
{
    // Sport Enum
    Sport Sport { get; set; }
}

Separate assembly / PlayerViewModelFactory.cs

[Export]
public sealed class PlayerViewModelFactory : IPlayerViewModelFactory
{
    [Import]
    private ILoggerFacade Logger { get; set; }

    public IPlayerViewModel Create(IPlayer player)
    {
        switch (player.Sport)
        {
            case Sport.Basketball:
                return new BasketballViewModel(Logger, player);

            case Sport.Football:
                return new FootballViewModel(Logger, player);

            // etc...

            default:
                throw new ArgumentOutOfRangeException("player");
        }
    }
}
Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top