IoC as service locator

Question

I have been wondering for quite some time on some issues while using dependency injection:

In a layered application, I normally inject repositories into the application service using constructor injection:

public class SomeService
{
    private IRepositoryA _repoA;
    private IRepositoryB _repoB;
    private IRepositoryC _repoC;

    public SomeService(IRepositoryA repositoryA, 
                       IRepositoryB repositoryB, 
                       IRepositoryC repositoryC
                       /* Other dependencies*/)
    {
        _repoA = repositoryA;
        _repoB = repositoryB;
        _repoC = repositoryC;
    }

    public void SomeMethod1()
    {
        //Do something with repo A and B
    }

    public void SomeMethod2()
    {
        //Do something with repo C
    }

    //Other methods
}

The problems with this are:

• When the service becomes complicated, the number of dependencies will grow more and more, and soon the constructor will become ugly with lots of parameters. It might be the service is doing too many things (violate SRP) and better be separated into several classes, but I think this situation is not uncommon so I'm curious how others solve this

• SomeMethod1() only uses repo A and B, but when client calls it, the service is created with all repositories, meaning it has more dependencies than it needs.

To address those 2 issues, I see some people have another abstraction, which is repository factory

public interface IRepositoryFactory
{
    T GetRepository<T>() where T : IRepository;
}

public class RepositoryFactory
{
    public T GetRepository<T>()
    {
        //Use IoC to return correct IRepository
    }
}

and inject factory into service class:

public class SomeService
{
    private IRepositoryFactory _factory;
    public SomeService(IRepositoryFactory factory)
    {
        _factory = factory;
    }

    public void SomeMethod1()
    {
        IRepositoryA repoA = _factory.GetRepository<IRepositoryA>();
        IRepositoryB repoB = _factory.GetRepository<IRepositoryB>();
        //Use repo A and B
    }

    public void SomeMethod2()
    {
        IRepositoryC repoC = _factory.GetRepository<IRepositoryC>();
        //Use repo C
    }

    //Other methods
}

This does solve the 2 problems above, but there are a few other issues:

• RepositoryFactory is using IoC as a service locator, which is considered as anti-pattern by many people
• RepositoryFactory has a dependency on IoC framework now, which might be undesirable in dependency injection (because as I understand, the application should have as little understanding about the IoC as they can, except for composition root, which is at the top of the application)

So is this a good approach to use?

Answer 1

Good Question!

When the service becomes complicated, the number of dependencies will grow more and more

In that case your dependencies are maybe not standalone dependencies but can be grouped into simple dependency-holding classes that can be used to pass into the constructor.

A good sign would be if more than one of your classes use both IRepositoryA an IRepositoryB in the constructor and also in some of their methods like in your SomeMethod1 method. You should be able to find a good name for the class that groups IRepositoryA and IRepositoryB. If not, consider going to tell us the real example so we can analyse better.

SomeMethod1() only uses repo A and B, but when client calls it, the service is created with all repositories, meaning it has more dependencies than it needs.

But that is okay because normally you want to make sure that at the time when the class is created you don't want the IRepositories to be changed anymore. They should be declared as immutable. That is a good practice.

If that's not the case and they shall be mutable, then you could make the IRepositoryC dependency beeing passed into SomeMethod2 as parameter. Currying / partial applying can help you to prevent code-duplication here when you call the method many times with the same IRepositoryC argument.

A different approach would be passing option-types into the constructor. Your class would then however have the responsibility to check in their methods if a given IRepository has already been provided or is still empty at the time of the method call.

So it depends a little on what you really want to do. There is imho no general answer to this problem.

Answer 2

The main problem with your second approach of injecting factories is that it removes one of the benefits IoC/dependency injection provides - You no longer know which repositories the class depends upon without digging through the code.

While this is not as 'bad' as creating the repositories itself, the class still needs to know how to request them from a factory. Also, you've added extra complexity by introducing the factory itself.

You are probably be better off following your instinct that the class is doing to much and separating it out into smaller chunks of responsibility.

I completely agree with the answer provided by @valenterry it's like most things, ymmv and it really depends on what you want to do. The second approach isn't exactly evil and is perfectly servicable if that's what you end up with.