Introducing Dependency Injection To A Large Brownfield Application
Published: 2021-01-07 18:21:51

This is the first journal entry [in a series](https://dev.to/onetomkenny/the-journey-of-a-brownfield-monolith-towards-solid-koo) that documents the conversion of a brownfield monolith towards SOLID programming principles.  See the original post for the story of our motivation.
 
# Proselytizing The Dependency Injection book

The book [Dependency Injection – Patterns Principles And Practices](https://www.amazon.com/Dependency-Injection-Principles-Practices-Patterns/dp/161729473X) by Steven Van Deursen and Mark Seemann is not the only book used to inform our move towards DI and Unit Testing but it’s the most important one.  Even if building greenfield, being informed by someone who has seen it all will help you to get the most out of these approaches.

## Spoiler Alert: We Only Did The First Six Chapters

The first six chapters of the book make the arguments for DI and provide a solid overall view of how to construct your business (domain) and repository layers.   The rest of the book focuses on tooling and, later, a very abstract Command/Query interface based approach.  If we had been building Greenfield I think we would have given that Command/Query approach a serious look but it was simply too radical for us to attempt on a brownfield project where cross-cutting-concerns were already implemented in a very different way.

## What We Did

The book advocates that any method making use of volatile dependencies should be in a “Service” class and those dependencies should be injected via constructor.

The biggest pitfall for us was making baby steps towards this goal when we should have made the big leap from the get-go. 

With the benefit of hindsight and experience, I will show you how you can break the important considerations down into manageable pieces that you won't have to go back and fix again later on.

Lets say that, like us, you have a business (domain) model expressed via classes that typically contain a bunch of properties, some “simple” helper methods and then methods that access the database.  

**For example, consider this class which models information about and actions on a Company:**

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/iqomae1am15h466f3sto.png)
 
It contains properties that model what a Company is. It also contains a Load() method that makes a call to a SQL database.  The IDBReader interface up top obligates the class to implement a BuildFromDBReader() method which is our standard way of reading database return columns into the properties of the class.  

**There are some problems with this class from a “Testability” standpoint:**

First, the Load() method is static and makes a direct call to a database (hidden inside BuildClassFromProcResults).  This will make any method that uses it difficult/impossible to unit test effectively.

Consider a method in some other class enforcing the requirement that a Company has a valid name:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/2gswgezno9pwzh8xup2c.png)

How do you write a unit test for this method when it contains that call to Company.Load()?   Inside the method we know there is a stored proc call that requires a real database behind it.   

You could pass a Company instance into the method, but then that method is liable to end up untestable for the same reasons that this one is.  

Looking back at the Company class again, the COLUMNS region is our way of tracking the names of the returned SQL columns from the Load() method.  But this means SQL specific implementation details are part of the class.   Even if you never plan on changing DB providers, why have those details cluttering up the class?  If you take pride in naming things well (which you should) then why would something called Company model aspects of a specific database implementation?

## “OK, but still, it doesn’t look that bad”.

That was our feeling for a long time.   But we started to notice some things over time.  

* There are often lots _**(lots)**_ of database related methods that start piling up in a class.
* For brevity, the example contains almost no “Business logic” code but once you add say five methods to express the behaviors of “Company” (which is after-all the point of having a business class), you are well on your way to “monster class” territory.
* So while this example looks pretty benign, you will eventually write 100 of these classes with all manner of interdependencies.
* Then when you need to add a feature or fix a bug, you will find yourself tripping over database specific code that has nothing directly to do with your problem but which is mixed up in the code you need to alter.  
* Throw in some caching, logging and security and you’ve built a puzzle the next guy will love spending whole days pondering.

## Separation Of Concerns

The big picture problem is that concerns need to be separated out.  The Company class should ideally tend towards being a [POCO class](https://en.wikipedia.org/wiki/Plain_old_CLR_object):  It should define the properties and behaviors of a Company but not include any database specific information (or information related to any other volatile dependency like caching, logging, security etc).

### Step 1:   Create a “Service” class.

Lets start by breaking up the Company class towards this ideal of a POCO object.  We do this by moving all the database specific code out into a CompanyService class:
 
![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/69ngbv9hoemswk6l08fa.png)

Notice the Load method is no longer static and the Update method has a Company parameter which contains the Company info being sent to the database.

**This leaves our original class a little lighter:**

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/k00x8zx2ma45nkes880p.png)

And CompanyChecker needs a bit of work to get the build working again:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/fvh2wdr5n3hv8y8eb6hw.png)

So what have we accomplished?   Nothing substantial yet, but we can now focus on that Load method as a Dependency and Inject that into the CompanyChecker code instead of making a new one inside the method.

### Creating A Dependency

The first step is to make the CompanyService methods implement an interface.  This will give us the  abstraction we need to support Mocking later on.

Start out by defining an ICompanyService interface in a new file:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/bsqwjnfvkx5zn2cg51ul.png)

It may seem like overkill to put this in its own file, but we have found through bitter experience that things tend to grow until the “one file” you are using becomes unwieldy.   If you are going to take a shortcut, putting too much in a file is probably the one you will come to regret the least as it’s the easiest to fix.

The next step is to make CompanyService implement this interface:
 
![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/ax93kcssdo6munlqkz39.png)

Finally, we need to define an interface method for each of the methods that do something with the database.  The easiest way to do this is to right click on the method and use the Quick Refactorings in VS to “pull” the method up into the interface:
 
![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/5ufwcs5mrzpqimhzs3jm.png)

Now your interface looks like this:
 
![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/tde1wx6g8l4thx7n7cpc.png)

Lets go back to CompanyChecker and alter it so that we Inject our CompanyService into it.  This will look familiar if you have read the Dependency Injection book.  We choose to use Constructor injection as per the advice of that book.  Method injection is also an option though it can often be cumbersome for consumers in a real world application.   Property injection is discouraged as it makes the resulting code harder to read and reason about – see sections 4.2 through 4.4 of the book for extensive discussion on these considerations which you want to understand before you go designing anything big.

**What we end up with is this:**

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/59a1yrcmbudcu1pva3c2.png)
 
CompanyChecker is rechristened CompanyCheckerService to reflect the fact that it makes use of volatile dependencies (of course, it always did - we just didn’t see it before). It implements an ICompanyCheckerService interface that looks like this:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/knql9ro3j2frtyp3411u.png)

The code inside the DI region (shown below) defines the _companyService property now used inside the CheckCompanyName method.

The complete class looks like this: 

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/yd53uoeqrgpkbrn76s6o.png)

For reference, here is what our folder structure for Company related code now looks like:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/b9qe6549vqucx41ah3xe.png)


Suddenly we are in a position to write a unit test.

Our Platform uses xUnit (and the VS Test Runner) along with Moq as our mocking library.  Choosing a good mocking library is important.  One weak spot that we have noticed in Moq is that its naming conventions make it easy to confuse Mocks with Stubs and Fakes.   Otherwise its working well for us.  You might want to look at FakeItEasy and other tools before you make a choice.

So far, we elect to put our unit tests in their own class in the same file with a naming convention of Test[ClassName].

The complete code to test the CheckCompanyName() method looks like this:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/z6hfol1ru5536gxmvqck.png)
 

So [Fact] is the attribute that marks the method as an xUnit test.    The test method name follows the common “Subject Scenario Result” naming scheme.

The first line uses the Moq library to create a mock object of type ICompanyService.  The second line mocks the Load() method so that it returns a Company with a value of null in its Name property.

The next line passes this mock into a new CompanyCheckerService.  Finally we call CheckCompanyName() and verify that an exception is thrown.

There is a lot more to Unit Testing than what is shown here and in the future I plan to write about our experiences in that area.   But in the end, writing good unit tests is a craft in and of itself.   

## TDD Digression

I should point out that if you were writing the functionality of “Company” from scratch, you could approach writing the CheckCompanyName() method with TDD coding practices.  Notice that once you have that ICompanyService interface defined, you could write the unit test above by mocking up the Load behavior before you even wrote an implementation for Load().   The idea is that you write your unit test so that it fails, and then work on CheckCompanyName() until the test passes.  

OK, so now we have a method that needs info from the database covered by a concise unit test that won’t break if you add new properties or methods to the Company class.  But Company still contains all that DBReader stuff plus those Database Column constants.  What now?

Now we take the next logical step which is to create a Repository service where all the database specific code resides, moving it out of Company.  That Repository service will then be injected into CompanyService.

Just as we did with CompanyService, we create a CompanyRepository class implementing an ICompanyRepositoryInterface.

Snapshots of the results look like this:

ICompanyRepository definition (in it’s own file):
 
![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/ftjx4pvop3qg0dbycy1d.png)

CompanyRepositoryService (implements Load and Update and has a method implementing our own BuildFromDBReader code to translate database return info into property values):

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/dhjywsaqpca5u9ikyzvt.png)

CompanyService now has an ICompanyRepositoryService object injected via the constructor which it uses for its Load and Update functionality:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/93wfpageojm8q7qhn9sj.png) 

Finally, we come to the Company class to find it stripped of everything except its properties and “simple” helper methods that don't require access to volatile dependencies:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/8b5stxhay14hjpip6dyy.png)

Finally, the files for all of this are currently organized like so:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/l305gk6qi3de8y26deok.png)

## “Hey, so CompanyService is nice, but what’s it really doing?”

When converting existing code over DI, it’s best to take a series of baby steps to keep trouble to a minimum.  There are three reasons for the existence of CompanyService:

* In real life a method like Update() might invalidate a cache or perform logging in addition to using the database.  The Load method might run security checks to see if the signed in user is allowed to call the method.   These will involve use of volatile dependencies that would also probably be injected into CompanyService alongside the RepositoryService.
* Indeed, a method like Update might involve so many dependencies or so much logic that you will find yourself breaking out a CompanyUpdateService just for that purpose.  What I’m showing you here is “an approach to developing” rather than “the answer”.
* Finally, when I was working on our Platform I took a shortcut early on based on my reading of the Dependency Injection book that got me into a bit of trouble:  I just renamed the original Company class to be CompanyService and then injected a RepositoryService into it, keeping the Load and Update methods where they were while moving the database specific code into the RepositoryService.   But then I found that I couldn’t new() up a CompanyService instance without feeding all sorts of dependencies into the constructor even if I didn’t want to call methods that used them.  I later had to go plowing through all my shiny new DI code to break out service classes to resolve these problems.

Honestly, its tedious breaking everything out in a way that feels almost compulsive, but separation of concerns will free you in the long run.   The time you spend searching for the code that does something will be more than offset by the confusion you wont experience because its not all mixed up with other concerns in some monster method or class.  And as my boss points out, breaking the contents of 400 files out into 2000 files doesn’t really create a searching problem in VS.   If you haven’t done it yet try “ctrl-t” in VS and start typing a class, method or file name.  There is no search time penalty when you implement a more verbose approach.

You may have read blog posts to the effect of “new() is Evil” or “Toxic”.   Those articles argue that new()-ing up objects with volatile dependencies will get you into trouble.   Our unfinished example is already in a state where you can new up a Company object without any “evil” or “toxicity” because now it’s just a simple class containing code that is true to its name.  You can also New() up the services provided you pass them the appropriate dependencies.  How you get those dependencies and where you set them up is a problem to be discussed in a future post.

In fact, many important design considerations that are covered in the Dependency Injection book are glossed over here.   You should read at least the first six chapters of that book.  **Like, now.**     

In the [next post](https://dev.to/onetomkenny/implementing-a-repository-layer-with-inverted-dependency-16k1) we will look at getting that RepositoryService into its own project where you are probably already thinking it should be.   Although our platform will probably never replace Microsoft SQL Server, we have an architecture that will eventually tell a reader of our business code next to nothing about the underlying data store (except perhaps the common ID property is a gives away that a Relational Database is involved).