Does a service-locator lead to more straightforward code in this situation?

Question

Assume I have a class like this:

public interface IService
{
    // Gets some data, possibly from a website / database
    List<string> GetStrings();
}

public class ItemViewModel : BaseViewModel
{
    // Some view-related data...

    // Dependency
    private readonly IService _service;

    // Property to which we bind something in the view that needs to have dynamic up-to-date data, for example, a combo box
    public List<string> Strings => _service.GetStrings();

    // Constructor injection
    public ItemViewModel(IService service)
    {
        _service = service;
    }        
}

Let's assume we need to have a list of these view models -- for instance let's have a class ItemsListViewModel, which takes care of these models, fills their data, removes them if needed, etc. How will this class create them?

I can think of 3 solutions how to deal with it:

1. ItemsListViewModel has a reference to IService and it can use this reference to create instances of ItemViewModel.
2. ItemsListViewModel has a reference to an abstract factory that communicates with the DI container and creates the instances. I mean this: https://github.com/ninject/Ninject.Extensions.Factory/wiki/Factory-interface
3. Changing ItemViewModel to:

public class ItemViewModel : BaseViewModel
{
    // Some view-related data...

    // Dependency
    private readonly IService _service;

    // Property to which we bind something in the view that needs to have dynamic up-to-date data, for example, a combo box
    public List<string> Strings => ServiceLocator.Get<IService>().GetStrings();     
}

My opinion on these solutions:

1. In this case a class has a dependency that doesn't actually need for anything but passing it further. This approach could lead to a super-class that has all the references and does nothing with them but passing them down.
2. In this case we need to have another level of unnecessary abstraction (some class creates another class in an abstract way).
3. No need for another abstraction, both classes have readable straightforward code. This seems like the best solution to me.

How would you solve this? Is there 'the best' solution?

Answer 1

I would go with 2, the abstraction is not unnecessary if you need to create a an instance determined at runtime and keep it loosely coupled.

The problem with 3 is that you're coupling ItemViewModel to the DI container.

Answer 2

Service Locator always leads to more straightforward code. Like global variables and "goto" operator. You just need to know what price you will pay for this simplicity. Your testing of ItemViewModel will be more complicated and changes to ServiceLocator may break your code. So, the best solution is to pass the necessary dependencies through the constructor or, possibly, the setter method.

Answer 3

Service locators are a readability trap.

When developing a specific class, using a service locator in that class seems to improve readability by cutting down on the copy/pastable injected dependencies.

The code becomes terser, there's less boilerplating (i.e. dependency property juggling), it's still easy to read what the code does, so that's a net plus, right?

The issue with service locators arises much later in the development process, when you're no longer looking at the specific class but are using it as a black box, i.e. a dependency in other parts of your codebase.

At this point, you want to be made aware explicitly about which dependencies your black box needs. That information is shown via the class' constructor arguments, which you've obfuscated by replacing them with a service locator.

Suddenly, you need to guess which dependencies you need to register in order to use your class, as opposed to logically being made aware of the required dependencies because you can't avoid the class constructor. For a sufficiently complex dependency graph, it's going to take a lot of shotgun debugging before you get the thing to work. If you're working in an enterprise-grade application where a debugging session is not done that quickly, you're going to lose a noticeable amount of time on your shotgunning.

---

In all fairness, IOC containers (which I do advocate for in general) usually rely on the service locator pattern, but they do so internally and try to keep it hidden from you as best as they can. The only thing you can't avoid is registering the dependencies, but at least it tends to be in a single location rather than manually peppered across your codebase.

I can't in good conscience call out a zero-tolerance on service location (because I'm not going to give up IOC containers), but I would urge you to not start using manual service location, as repeated usage will detract from your codebase quality even if it initially looks like it's making things better.