Reducing Dependency Injection Code Smell: An Introduction

February 10, 2020 by Michael

At some point in your career as a software developer you will inevitably run into dependency injection. This discovery will most likely lead you to using the IoC container of your choice (e.g. Lamar, StructureMap, or Ninject) and enjoying the benefits that come along with it.

At first everything will be wonderful: you and dependency injection will become the best of friends. You’ll have loosely coupled code powered by your IoC container and unit testing will be a breeze. Nothing could be better.

However, it won’t be long until you start to wonder if your new best friend has started to lead you astray. You’ll find yourself adding your eighth or ninth parameter to a constructor all the while questioning your life decisions. This can’t be right can it? I mean… look at this:

public AccountService(
    bool allowOverdrafts,
    INotificationService notificationSvc,
    IOverdraftService overdraftSvc,
    IBouncedCheckService bouncedCheckSvc,
    IEmailService emailSvc,
    IValidationService validationSvc,
    IAccountRepository accountRepo,
    IPersonRepository personRepo,
    IClock clock,
    ILogger logger)
{
    _allowOverdrafts = allowOverdrafts;
    _notificationSvc = notificationSvc;
    _overdraftSvc = overdraftSvc;
    _bouncedCheckSvc = bouncedCheckSvc;
    // Code removed for brevity
    _logger = logger;
}

How is it that a class that is supposed to do one thing (Single Responsibility Principle) needs 10 different things injected into it? Is dependency injection an anti-pattern?

Using an IoC container makes it incredibly easy to implement dependency injection: almost too easy. If you’ve landed on this page, you’re probably looking for a better way to handle over injection. If that’s the case, read on my dear friend… read on.

Refactoring Dependency Injection

The first step towards cleaner code is admitting that you have a dependency injection problem. If you’re here, you’ve already done that. The next step is identify alternative solutions to your over injection problem.

Here is a summary of the strategies that will be presented over the next few posts:

  • Basic Refactoring
  • Facade Services (a.k.a. Aggregate Services)
  • Decorators
  • Interceptors (Aspect Oriented Programming)
  • Message Broker

This post will focus on basic refactoring. Facade Services, decorators, interceptors, and the message broker strategy will be covered in later posts.

Basic Refactoring

Before diving into more complex solutions to the dependency injection problem, you should first make sure that your class isn’t violating the Single Responsibility Principle. This isn’t as obvious as it may seem. Consider the following method from an Account class:

public void Process(Check check)
{
    try
    {
        if (!check.IsValid)
        {
            _logger.Error("Invalid transaction {@Transaction}", check);
            throw new ArgumentOutOfRangeException(nameof(check), check, "Invalid transaction");
        }

        if(!(HasSufficientFunds(check.Amount) || _allowOverdrafts))
        {
            _emailSvc.SendNotification("Check bounced, insufficient founds an overdrafts are not allowed");
            _bouncedCheckSvc.ApplyPenalty(this);

            return;
        }

        _balance -= check.Amount;

        if (_balance < 0)
        {
            _emailSvc.SendNotification("Account over withdrawn");
            _overdraftSvc.ApplyPenalty(this);
        }
    }
    catch (Exception ex)
    {
        _logger.Error("The following exception is being logged: {Message}", ex.Message);
        throw;
    }
}

Fundamentally this method should do just one thing: update the account's balance. Looking closely you'll see that as written, this method:

  • Validates input
  • Logs errors / exceptions
  • Sends emails
  • Triggers the bounced check business logic
  • Triggers the overdraft business logic
  • Updates the account balance

You could argue that some of these things are fundamentally required for production ready code. While true, I would counter that there are better solutions that make your code more readable, easier to maintain, and help you adhere to the DRY principle.

Input Validation

Generally speaking, validating your input is a good thing. It's something you really should be doing. The question is, "where should input validation happen?"

If you are developing a web based application (anything with a client / server model really), you have at least two opportunities to validate input before getting into your business logic: client-side and server side. Most front-end frameworks have some degree of built-in support for input validation. Use it whenever possible. Server-side you should be validating input before handing it of to your business logic. You can see one way to automate that process here.

If you still believe input validation should happen in your core business logic, I'm willing to cede the point. However, side effects (emails, logging, etc...) generated by error conditions in your business logic should not be emitted by the business logic itself.

Exception Logging

Looking at the code you can see that it does nothing more than log the exception and re-throw it. If your code cannot actually do anything about the exception, don't catch it. Let something higher up the object hierarchy handle the exception and any related logging.

Sending Emails / Notifications

Requirements that read something like "send an email to user when [something] happens" are pretty common. It's also pretty common for those requirements to evolve over time. Perhaps instead of using email you now need to send a text message and an email.

In my example, generating these side effects from the Account object has tightly coupled the account logic and the notification logic: two separate concerns. We can help keep the Account object highly cohesive and loosely coupled if we extract that logic and let an orchestration object respond to a result object. I'll demonstrate this concept in my refactored code below.

Triggering Related Workflows

It's tempting to explicitly trigger related workflows based on clearly defined business rules. However, doing this means you are making a lot of assumptions about the related workflow. Most notably you're assuming that it will not change and that it performs well enough to scale along with your own code. These are dangerous assumptions.

Much like generating side effects, these responsibilities are best handled outside of your singularly focused business logic. Taking this approach will significantly reduce your need to use dependency injection.

Updating Account Balance

This is it: the one thing your method should actually be doing. Let's see how we can refactor the above code to makes things a bit more manageable...

The Refactored Method

The complete code for this example can be found in this GitHub repository. Only the relevant code is reproduced below.

public ProcessResult Process(Check check)
{
    ProcessResultBuilder builder = new ProcessResultBuilder();

    if (!(HasSufficientFunds(check.Amount) || _allowOverdrafts))
        builder
            .AddErrorMessage("Check bounced, Check bounced, insufficient founds an overdrafts are not allowed")
            .SetStatus(ProcessStatus.Bounced);

    if (builder.HasErrorMessages)
        return builder.UsingBalance(_balance);

    _balance -= check.Amount;

    if (_balance < 0)
        builder.SetStatus(ProcessStatus.OverWithdrawal);

    return builder.UsingBalance(_balance);
}

Notice that the refactored code no longer calls any external services: no dependency needs to be injected.

Two key changes were made to this method. First, a result object is used to return all necessary information to the calling object. The method itself no longer needs to generate notifications nor does it need to do any logging. The second change is that input validation and the superfluous exception handling were removed. This allows the method to focus on the one thing it should be doing: managing the account.

If you insist on the extra error checking and input validation, that could be implemented as follows:

public ProcessResult Process(Check check)
{
    ProcessResultBuilder builder = new ProcessResultBuilder();

    try
    {
        if(!check.IsValid)
            builder.AddErrorMessage("Invalid check");

        if(!(HasSufficientFunds(check.Amount) || _allowOverdrafts))
            builder
                .AddErrorMessage("Check bounced, Check bounced, insufficient founds an overdrafts are not allowed")
                .SetStatus(ProcessStatus.Bounced);

        if(builder.HasErrorMessages)
            return builder.UsingBalance(_balance);

        _balance -= check.Amount;

        if(_balance < 0)
            builder.SetStatus(ProcessStatus.OverWithdrawal);

        return builder.UsingBalance(_balance);
    }
    catch (Exception ex)
    {
        return builder
            .WithException(ex)
            .UsingBalance(_balance);
    }
}

Conclusion

This post focused on a few things you can do to minimize your use of dependency injection. In simple cases this could be enough to get you on the right track. However, in more complicated scenarios all the above refactoring accomplishes is deferring responsibilities to some other object.

The next time I discuss this topic I will focus on using interceptors and aspect oriented programming to address cross cutting concerns. AOP allows you to handle common issues like input validation and exception logging in a way that allows you to scale back on dependency injection and keeps your code relatively DRY.

Helpful Links

I found the following links helpful while research this blog post:

  • http://blog.ploeh.dk/2018/08/27/on-constructor-over-injection/
  • https://stackoverflow.com/questions/19526474/aop-implemented-with-the-decorator-pattern-over-a-generic-repository
  • http://structuremap.github.io/interception-and-decorators/

You can find the source code for this post on GitHub.

Discussion

Leave a Reply

Your email address will not be published. Required fields are marked *