2

The question is about choosing the appropriate design for the scenario described below. This is a repost from https://stackoverflow.com/questions/51940180/unity-injection-with-too-many-constructor-parameters where it was suggested to put the question here.

The question seems to cause a severe controversy even among some of the best-known C# gurus. In fact, it is far beyond C# and it falls more into pure computer science. The question is based on the well-known "battle" between a service locator pattern and pure dependency injection pattern: https://martinfowler.com/articles/injection.html vs http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/ and a subsequent update to remedy the situation when the dependency injection becomes too complicated: http://blog.ploeh.dk/2010/02/02/RefactoringtoAggregateServices/

I read everything that I could find on the subject and even contacted several well-known C# gurus directly, but yet it is still unclear what would be the best choice.

Development setup and requirements

  1. We use Unity for DI / IOC. As such we never create classes directly and inject only interfaces through constructors.
  2. We use Moq with MockBehavior.Strict for all unit tests to ensure that we always get expected behavior and no hidden surprises.
  3. We use partial mock for integration tests where all external services are mocked in some way and all (or almost all) internal services are real.
  4. We use a single composition root where all interfaces are registered.
  5. We want to simplify the design and decrease the maintenance time / cost.

Business setup

We have a large (50 – 100+) collection of "rules", which I call micro services, because each rule is wrapped into a single service. If you have a better name, please "apply" it when reading. Each of them operates on a single object, let's call it quote. However, a tuple (context + quote) seems more appropriate. Quote is a business object, which gets processed and serialized into a database and context is some supporting information, which is necessary while quote is being processed, but is not saved into the database. Some of that supporting information may actually come from database or from some third-party services. This is irrelevant. Assembly line comes to mind as a real-world example: an assembly worker (micro service) receives some input (instruction (context) + parts (quote)), processes it (does something with parts according to instruction and / or modifies instruction) and passes it further if successful OR discards it (raises exception) in case of issues. The micro services eventually get bundled up into a small number (about 5) of high-level services. This approach linearizes processing of some very complex business object and allows testing each micro service separately from all others: just give it an input state and test that it produces expected output.

Here is where it gets interesting. Because of the number of steps involved, high-level services start to depend on many micro services: 10-15+ and more. This dependency is natural, and it just reflects the complexity of the underlying business object. On top of that micro services can be added / removed nearly on a constant basis: basically, they are some business rules, which are almost as fluid as water. Somebody may state that such high-level services violate Single Responsibility Principle. Well, if I have to apply, let’s say, 15+ "rules" in order to create a quote, then the implementation of IQuoteCreateService.CreateQuote has to apply all of them, while performing only a single task, creating quote!

That severely clashes with Mark's recommendation above: if I have 15+ effectively independent rules applied to a quote in some high-level service, then, according to the third blog, I should aggregate them into some logical groups of, let's say no more than 3-4 instead of injecting all 15+ via constructor. But there are no logical groups! While some of the rules are loosely dependent, most of them are not and so artificially bundling them together will do more harm than good.

Throw in that the rules change frequently, and it becomes a maintenance nightmare: all real / mocked calls must be updated every time the rules change.

And I have not even mentioned that the rules are US state dependent and so, in theory, there are about 50 collections of rules with one collection per each state and per each workflow. And while some of the rules are shared among all states (like "save quote to the database"), there are a lot of state specific rules.

Here is a very simplified example.

Quote - business object, which gets saved into database.

public class Quote
{
    public string SomeQuoteData { get; set; }
    // ...
}

Micro services. Each of them performs some small update(s) to quote. Higher level services can be also built from some lower level micro services as well.

public interface IService_1
{
    Quote DoSomething_1(Quote quote);
}
// ...

public interface IService_N
{
    Quote DoSomething_N(Quote quote);
}

All high-level and micro services inherit from this interface. It is convenient because the low-level implementation: QuoteProcessor provides some common tasks, like call for data validation, perform finalizer tasks (if necessary), etc... This is irrelevant to the question but explains why micro services also inherit from this interface.

public interface IQuoteProcessor
{
    List<Func<Quote, Quote>> QuotePipeline { get; }
    Quote ProcessQuote(Quote quote = null);
}

// Low level quote processor. It does all workflow related work.
public abstract class QuoteProcessor : IQuoteProcessor
{
    public abstract List<Func<Quote, Quote>> QuotePipeline { get; }

    public Quote ProcessQuote(Quote quote = null)
    {
        // The real code performs Aggregate over QuotePipeline.
        // That applies each step from workflow to a quote.
        return quote;
    }
}

One of high level "workflow" services:

public interface IQuoteCreateService
{
    Quote CreateQuote(Quote quote = null);
}

and its actual implementation where we use many of low level micro services.

public class QuoteCreateService : QuoteProcessor, IQuoteCreateService
{
    protected IService_1 Service_1;
    // ...
    protected IService_N Service_N;

    public override List<Func<Quote, Quote>> QuotePipeline =>
        new List<Func<Quote, Quote>>
        {
            Service_1.DoSomething_1,
            // ...
            Service_N.DoSomething_N
        };

    public Quote CreateQuote(Quote quote = null) => 
        ProcessQuote(quote);
}

Issues

There are two main ways to achieve DI:

Standard approach is to inject all dependencies through constructor:

    public QuoteCreateService(
        IService_1 service_1,
        // ...
        IService_N service_N
        )
    {
        Service_1 = service_1;
        // ...
        Service_N = service_N;
    }

And then register all types with Unity:

public static class UnityHelper
{
    public static void RegisterTypes(this IUnityContainer container)
    {
        container.RegisterType<IService_1, Service_1>(
            new ContainerControlledLifetimeManager());
        // ...
        container.RegisterType<IService_N, Service_N>(
            new ContainerControlledLifetimeManager());

        container.RegisterType<IQuoteCreateService, QuoteCreateService>(
            new ContainerControlledLifetimeManager());
    }
}

Then Unity will do its "magic" and resolve all services at run time. The problem is that currently we have about 50-100 such micro services and the number is expected to increase. Subsequently some of the constructors are already getting 10-15+ services injected. This is inconvenient to maintain, mock, etc...

Sure, it is possible to use the idea from here: http://blog.ploeh.dk/2010/02/02/RefactoringtoAggregateServices/ However, the micro services are not really related to each other and so bundling them together is an artificial process without any justification. In addition, it will also defeat the purpose of making the whole workflow linear and independent (a micro service takes a current "state", then preforms some action with quote, and then just moves on). None of them cares about any other micro services before or after them.

An alternative idea seems to create a single "service repository" or service locator:

public interface IServiceRepository
{
    IService_1 Service_1 { get; set; }
    // ...
    IService_N Service_N { get; set; }

    IQuoteCreateService QuoteCreateService { get; set; }
}

public class ServiceRepository : IServiceRepository
{
    protected IUnityContainer Container { get; }

    public ServiceRepository(IUnityContainer container)
    {
        Container = container;
    }

    private IService_1 _service_1;

    public IService_1 Service_1
    {
        get => _service_1 ?? (_service_1 = Container.Resolve<IService_1>());
        set => _service_1 = value;
    }
    // ...
}

Then register it with Unity and change the constructor of all relevant services to something like this:

    public QuoteCreateService(IServiceRepository repo)
    {
        Service_1 = repo.Service_1;
        // ...
        Service_N = repo.Service_N;
    }

The benefits of this approach (in comparison to the previous one) are as follows:

All micro services and higher-level services can be created in a unified form: new micro services can be easily added / removed without the need to fix constructor call for the services and all unit tests. Subsequently, maintenance and complexity decreases.

Due to interface IServiceRepository, it is easy to create an automated unit test, which will iterate over all properties and validate that all services can be instantiated, which means that there will be no nasty run time surprises.

The problem with this approach is that it starts looking a lot like a service locator, which some people consider as an anti-pattern: http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/ and then people start to argue that that all dependencies must be made explicit and not hidden as in ServiceRepository.

Yet a few more ideas were provided in the answers to the original question (in the link above). They all centered around the idea of using IEnumerable to pass as parameters array or as the collection of rules to be applied. I personally feel that using parameter array to pass services into constructor will do more harm than good due to the need for maintaining each constructor call in sync and without compiler to the rescue.

Question

I want to decrease complexity associated with injecting too many parameters through constructor. The rules (or parameters) are fairly independent, so I don't have a logical reasoning to bundle them up to decrease the number of parameters for the top class(es). From the other side, the business setup does require that many nearly independent rules must be applied at the level of the top business object(s) (quote, for example).

  • 1
    The phrase "What design pattern shall I apply" may unfortunately attract a lot of down votes because it is often comes from people who don't understand the nuances of what they're trying to accomplish. That does not appear to be the case here, but you may want to edit the actual question at the bottom to better express your concerns. (I'm assuming you don't want a one line answer "use Pattern XYZ"). See Should I follow patterns strictly? for more info – Dan Pichelman Aug 23 '18 at 17:55
  • 2
    The most significant factor in your decision making is not what Martin Fowler, Mark Seeman or anyone else on the Internet says about what the "right" way of doing things is; it's going to be: what specific problem are you trying to solve? Can you tell us what that problem is? If the problem boils down to "too many constructor parameters," the obvious solution would be to package them up in a DTO. – Robert Harvey Aug 23 '18 at 18:01
  • @RobertHarvey I want to decrease complexity associated with injecting too many parameters. The rules (or parameters) are fairly independent, so I don't have a logical reasoning to bundle them up to decrease the number of parameters for the top class(es). The legacy code has a lot of such "monster" classes with 15+ parameters, where everything, including unit tests is tightly coupled together. This is a nightmare to change as the developer has to work through many layers of interface calls and conditional branches (yes, some rules are internally bundled up and resolved conditionally!) – Konstantin Konstantinov Aug 23 '18 at 18:40
  • There are several things that come to mind that might help. You could "protocolize" your parameters like Protocol Buffers does. You could make better use of things like first class functions, Factory methods, the Strategy Pattern, etc. None of those things has much to do with DDD, however, nor with Dependency Injection containers per se. Your problem seems much more fundamental than that. – Robert Harvey Aug 23 '18 at 18:44
  • @DanPichelman I updated the question with more precise goal. – Konstantin Konstantinov Aug 23 '18 at 19:46
1

This piece of code caught my attention:

public override List<Func<Quote, Quote>> QuotePipeline =>
    new List<Func<Quote, Quote>>
    {
        Service_1.DoSomething_1,
        // ...
        Service_N.DoSomething_N
    };

The fact that you're taking a list of discrete elements and immediately putting them in a list suggests that there is an implicit business concept here that isn't embodied in a class. If it were me, I'd have a class to represent the pipline and inject either the pipeline itself or a factory that will allow the caller to create one.

If you're writing a factory, it is "OK" to pass in the Unity Container itself, since dependency and lifespan management is part of a factory's single responsibility. This way you are getting the benefits of the service locator pattern without breaking the rules too badly for IoC.

When you need to unit test, you can just substitute a different pipeline factory that returns mocks.

So, first we define the pipeline:

public class QuotePipeline : List<Func<Quote,Quote>>
{
    public Quote Execute(Quote quote)
    {
        foreach (var f in this) quote = f(quote);
        return quote;
    }
}

Now write a factory. Unity will always inject itself automatically if your class has IUnityContainer as a constructor argument.

class QuotePipelineFactory : IQuotePipelineFactory
{
    protected readonly IUnityContainer _container;

    public QuotePipelineFactory(IUnityContainer container)
    {
        _container = container;
    }

    public QuotePipeline GetPipeline()
    {
        var p = new QuotePipeline();

        var d1 = _container.Resolve<Service_1>();
        p.Add( q => d1.DoSomething_1(q) );

        var d2 = _container.Resolve<Service_2>();
        p.Add( q => d2.DoSomething_2(q) );

        return p;
    }
}

Then inject the factory and retrieve the pipeline itself:

public class QuoteCreateService : IQuoteProcessor
{
    protected readonly IQuotePipelineFactory _quotePipelineFactory;

    public QuoteCreateService(IQuotePipelineFactory quotePipelineFactory)
    {
        _quotePipelineFactory = quotePipelineFactory;
    }

    public Quote CreateQuote(Quote quote)
    {
        var p = _quotePipelineFactory.GetPipeline();
        return p.Execute(quote);
    }
}

Notice now there is only one dependency, and it can support any number of quote processors.

If your quote pipelines are always the same, you can of course store it as a single instance within the factory, which will be appropriately lifetime-scoped by Unity.

If the pipelines differ, you can always add input arguments to GetPipeline() and then possibly use a strategy pattern to pick and choose which processors to include in the pipeline.

0

I personally don't like injection through constructors at all. That's because classes will need injected classes that only they and nobody else cares about, but now anyone creating an instance of the class has to perform these injections, so they need them injected to their constructors as well, and so on and so on.

Much better if each class looks after itself. Have some factory, or have singletons that return a replacement if needed, and everyone collects whatever injected objects they need. Seriously, why would you want an injected class in your constructor that is used by some other instance seven levels away?

  • 1
    This is where a DI framework, where you wire all these constructors up once, is sometimes very practical. You don’t manually create objects in runtime code, you might say it’s “injection all the way down”. Users of A should never care about A’s dependencies because they never create A in the first place, they just get one through their constructor. – axl Aug 25 '18 at 14:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.