Hidden Dependencies as a Smell

/, Best Practices, CodeProject, Design, Refactoring/Hidden Dependencies as a Smell

Hidden Dependencies as a Smell

Mark Seemann has written a nice post  “Service Locator violates encapsulation”. The name of the post speaks for itself that it’s about a pattern (anti-pattern) named Service Locator. When a programmer arbitrarily inside the code base calls for the IoC-container to resolve a dependency of an object – he uses a Service Locator anti-pattern. Mark provides the following example:

public class OrderProcessor : IOrderProcessor
{
    public void Process(Order order)
    {
        var validator = Locator.Resolve<IOrderValidator>();
        if (validator.Validate(order))
        {
            var shipper = Locator.Resolve<IOrderShipper>();
            shipper.Ship(order);
        }
    }
}

As we can see the encapsulation of the type OrderProcessor is broken due to two hidden dependencies which silently resolved inside the Process method. These dependencies are hidden from a caller and that can lead to runtime exceptions in case a caller hasn’t set up the appropriate IoC-container with the required dependencies. As a solution to this problem, Mark suggests to move the resolving of the dependencies to the constructor of an object.

public class OrderProcessor : IOrderProcessor
{
    public OrderProcessor(IOrderValidator validator, IOrderShipper shipper)

    public void Process(Order order)
}

So now a caller is aware of what actually OrderProcessor requires. Sounds reasonable and actually I agree with that.

But there is still some room for dependency-hiding approach in my opinion. Consider a WPF-application which in almost every ViewModel requires IEventAggregator, IProgress, IPromptCreator dependencies. To reveal the meaning of the two latter interfaces I’ll say that IProgress implementation should provide the functionality of accepting a chunk of long-running code and showing a View with a progress bar, IPromptCreator provides the ability to open, well… prompts. Now imagine that there are some ViewModels which require in addition two (or maybe even three) dependencies for creating an instance of a Model. This is how a ViewModel might look like with so many dependencies:

public class PaymentViewModel: ICanPay
{
    public PaymentViewModel(IPaymentSystem paymentSystem, 
                            IRulesValidator rulesValidator, 
                            IEventAggregator aggregator, 
                            IProgress progress, 
                            IPromptCreator promptCreator)
    public void PayFor(Order order)
}

What a mess! There is too much noise in the constructor declaration. Just two dependencies actually carry useful information from the viewpoint of a real business value.

If we use, say, MEF for dependency injection, then we could do the following:

[Export]
public class PaymentViewModel : ICanPay
{
    [Import]
    protected IEventAggregator aggregator;
    [Import]
    protected IProgress progress;
    [Import]
    protected IPromptCreator promptCreator;

    public PaymentViewModel(IPaymentSystem paymentSystem, 
                            IRulesValidator rulesValidator)
    {
    }

    public void PayFor(Order order)
    {
        //use aggreagtor, progress, promptCreator
    }
}

We moved dependencies out of the constructor to the fields declaration and marked them by the attribute Import. Despite of that we don’t call for the IoC-container (though, MEF is not a an IoC-container) directly, we still hide dependencies as it was in Mark’s example. Nothing really changed. Why I think this code is not so bad? For several major reasons:

  • ViewModels are not business entities, they are just chunks of glue code, no one cares of their’s dependencies too much.
  • ViewModels are not public API’s and they are not reusable (in most cases)
  • Because of two previous statements it may become just an agreement between team-members that ViewModels have those utility dependencies and that’s it.
  • These utility dependencies are declared as protected, what allows us in tests to create a ViewModel-class which inherits from the PaymentViewModel and then replace those dependencies by mocks, cause we have access to those fields. So we don’t lose the opportunity to cover PaymentViewModel by unit-tests. It was necessary in Mark’s example (where Service Locator is used) to wire up the IoC-container in unit-test projects in order to mock or stub those dependencies and such a practice could become a pain for unit-testing process.

Conclusion

As I’ve always said, there are no single right answers or single statements which are always true. Generally speaking, we should avoid Service Locator, because it breaks encapsulation, as Mark said in his article. Before using Service Locator consider the potential harm it can bring to a system. If you are sure, that implicit dependencies resolving is irrelevant for clients (your team folks) and there’s no potential damage for a system, then go on.

By | 2017-09-13T08:48:13+00:00 October 27th, 2015|.NET, Best Practices, CodeProject, Design, Refactoring|4 Comments

About the Author:

I'm thankful enough for that I love what I do. I began my career as a postgraduate student participating in Microsoft ImagineCup contest. I've been working with .NET platform since 2003. I've been professionally architecting and implementing software for nearly 7 years, primarily based on .NET platform. I'm passionate about building rich and powerful applications using modern technologies. I'm a certified specialist in Windows Applications and Service Communication Applications by Microsoft. "If it's work, we try to do less. If it's art, we try to do more." - Seth Godin. What I can say is that software is my art.
  • If you have too many dependencies, it’s a smell. Moving them out of the constructor and to properties or fields doesn’t solve the problem; it only adds to the problem.

    The PaymentViewModel class still needs IEventAggregator in order to work, but now you’ve made it possible to create an instance of it without supplying the required dependencies. This causes the same problem with poorly advertised pre-conditions as we’ve seen before. At the very least, it’s no longer Poka-yoke code: http://blog.ploeh.dk/2011/05/24/Poka-yokeDesignFromSmelltoFragrance

    If a constructor takes too many arguments, there’s at least one benefit from that: it makes it quite clear that the Single Responsibility Principle is in jeopardy: http://stackoverflow.com/a/2420245/126014

    • Poka-yoke article series is great. By the way, I translated into Russian almost all of them and shared with the Russian audience (of course with links to your blog).
      As to our discussion.
      Actually, we finally aggregated those three utility dependencies into a single object and injected it through a constructor in the production. While I understand that we violate encapsulation by hiding those dependencies, I’m still not convinced that there’s much harm, cause in our case no one ever creates ViewModels explicitly, we have a single “entry point” where ViewModels created by the IoC-container and ViewModels are never reused in our solution. Despite of that I’m thinking this way, as I’ve said we inject utility dependencies through constructors, but I still consider them more like a noise then a valuable information. I’m not sure if we have some sort of a trick to make those dependencies a part of an MVVM-framework, this approach sounds reasonable.
      As to “Refactoring to Aggregate Services” – actually we can do that, but I’m not sure if this will make a difference. It seems to me, that the best approach is to embed those utilities into the MVVM-framework, so people have to just know as a precondition of using that framework, that there are those dependencies. We should just somehow wire up all that stuff correctly.

  • Something is strange.

    In your first example, why would you call Locator.Resolve to create instances of IOrderValidator and IOrderShipper every-time?
    Are these supposed to be created each time or are they singletons? Do they really require DI?

    I think your statement
    ” in case a caller hasn’t set up the appropriate IoC-container with the required dependencies”
    depends on the case.
    Otherwise it’s like saying generally IoC is bad because someone might forget registration.

    Also, the fact that sometimes the number of dependencies can grow and c-tor parameters become large, it might be a sign you need to do some refactoring on dependencies.
    If you have many objects which have same dependencies, you can wrap the dependencies in one, and have this one as just one dependency for your objects.

  • Another thing:
    Personally I think it’s OK if you have some “hidden” dependencies as long as these are actually singletons.
    As long as you don’t abuse it of course.