TDD implementation rules in the old project

The article “Sliding responsibility of the Repository pattern” raised several questions that are very difficult to answer. Do I need a repository, if it is completely impossible to abstract from technical details? How complicated can the repository be so that its writing remains appropriate? The answer to these questions differs depending on the emphasis that is being made when developing systems. Probably the most difficult question: is a repository necessary at all? The problem of "fluid abstraction" and the increase in coding complexity with an increase in the level of abstraction make it impossible to find a solution that would satisfy both camps. For example, in reporting, intention design leads to the creation of a large number of methods for each filter and sorting, while a generic solution creates a large overhead on coding. You can continue indefinitely ...

For a more complete picture, I looked at the problem of abstractions from the side of applying them in the already ready code, in the legacy code. The repository, in this case, we are only interested in, as a tool for achieving high-quality and meaningless code. Of course, this pattern is not the only thing necessary for the application of TDD practices. After eating “tasteless food” in several large projects and watching what works and what doesn't, I brought out for myself a few rules that help me follow TDD practices. I am pleased to hear constructive criticism and other techniques for implementing TDD.

Foreword

Some may notice that in the old project it is impossible to apply TDD. There is an opinion that different types of integration tests (UI-tests, end-to-end) are more suitable for them, since it’s too hard to figure out the old code. You can also hear that writing tests before encoding itself only leads to loss of time, since we may not know how the code will work. I had to work in several projects where I was limited only to integration tests, considering that unit tests are not indicative. At the same time, a lot of tests were written, they launched a bunch of services, etc., etc. As a result, only one person could actually figure this out, who actually wrote them.

During my practice I managed to work on several very large projects, where there was a lot of legacy code. In some there were tests, in others they were only going to implement it. I myself managed to raise 2 large projects. And everywhere I somehow tried to apply the TDD approach. In the initial stages of understanding TDD, it was perceived as Test First development. But the further, the more clearly the differences between this simplified understanding and the current presentation, called shortly BDD, were visible. Whatever language is used, the main points that I called the rules remain similar. Someone may find parallels between the rules and other good code writing principles.
')

Rule 1: Use Bottom-Up (Inside-Out)

This rule is more relevant to the way software is analyzed and designed when new pieces of code are embedded in an existing project.

When you are designing a new project, it is absolutely natural to represent the entire system. At this stage, you control the set of components and the future flexibility of the architecture. Therefore, you can write modules that are conveniently and better integrated with each other. Such a top-down approach allows you to perform a good upfront design of the future architecture, describe the necessary guidelines and have a holistic view of what you want in the end. After some time, the project turns into what is called the legacy code. And here the most interesting begins.

At a stage when a new functionality needs to be built into an existing project with a bunch of modules and dependencies, it can be very difficult to put them all in my head so that the design is correct. The other side of this problem is the amount of work needed to complete such a task. Therefore, in this case the approach from below will be more effective. In other words, first you create a complete module that solves the necessary problem, and then embed it into the existing system, making only the necessary changes. In this case, you can vouch for the quality of this module, since it is a complete unit of functionality.

It should be noted that the approaches are not so simple. For example, when designing a new functional in the old system, you will not use both approaches. In the initial analysis, you still need to evaluate the system, then lower it to the module level, implement it, and then return to the system level again. In my opinion, the main thing here is not to forget that the new module should be a complete functionality and be independent, as a specific tool. The clearer it will be to stick to this approach, the fewer changes will be made to the old code.

Rule 2: Test only modified code

When working with an old project, there is absolutely no need to write tests for all possible scenarios of the method / class. Moreover, you may not be aware of some scenarios at all, since There may be a lot of them. The project is already in production, the client is satisfied, so you can relax. In the generalized case, in such a system, only your changes introduce problems. Therefore, only they should be tested.

Example

There is an online store module that creates a basket of selected items and saves it to the database. The concrete implementation does not bother us. How done so done - this is the legacy code. Now we need to introduce a new behavior here: send a notification to the accounting department in the case when the basket value exceeds $ 1000. This is the code we see. How to implement a change?

public class EuropeShop : Shop { public override void CreateSale() { var items = LoadSelectedItemsFromDb(); var taxes = new EuropeTaxes(); var saleItems = items.Select(item => taxes.ApplyTaxes(item)).ToList(); var cart = new Cart(); cart.Add(saleItems); taxes.ApplyTaxes(cart); SaveToDb(cart); } } 

According to the first rule, the changes should be minimal and atomic. We are not interested in downloading data, not interested in accounting for taxes and saving to the database. But we are interested in already calculated basket. If there was a module that does what is necessary, then it would perform the necessary task. Therefore, doing so.

 public class EuropeShop : Shop { public override void CreateSale() { var items = LoadSelectedItemsFromDb(); var taxes = new EuropeTaxes(); var saleItems = items.Select(item => taxes.ApplyTaxes(item)).ToList(); var cart = new Cart(); cart.Add(saleItems); taxes.ApplyTaxes(cart); // NEW FEATURE new EuropeShopNotifier().Send(cart); SaveToDb(cart); } } 

Such a notifier is working by itself, can be tested, and the changes made to the old code are minimal. This is exactly what the second rule says.

Rule 3: We test only requirements

In order not to bury oneself in the number of scenarios that require unit testing, think about what is actually needed from the module. Write first for the minimum set of conditions that can be presented as module requirements. The minimum set is such that, when added to which a new one, the behavior of the module almost does not change, and when removed, the module becomes inoperative. Very well helps to put brains on the right rails approach BDD.

Just imagine how other classes that are clients of your module interact with it. Do they need to write 10 lines of code to configure your module? The easier the communication between parts of the system, the better. Therefore, it is better to select the modules responsible for something specific from the old code. This is where SOLID comes to the rescue.

Example

Now let's see how everything about what he wrote above will help us with the code. First select all the modules that are only indirectly related to the creation of the basket. This is how the responsibility of the modules is distributed.

 public class EuropeShop : Shop { public override void CreateSale() { // 1) load from DB var items = LoadSelectedItemsFromDb(); // 2) Tax-object creates SaleItem and // 4) goes through items and apply taxes var taxes = new EuropeTaxes(); var saleItems = items.Select(item => taxes.ApplyTaxes(item)).ToList(); // 3) creates a cart and 4) applies taxes var cart = new Cart(); cart.Add(saleItems); taxes.ApplyTaxes(cart); new EuropeShopNotifier().Send(cart); // 4) store to DB SaveToDb(cart); } } 

And so they can be distinguished. Such changes at once are, of course, impossible in a large system, but they can be made gradually. For example, when changes concern a tax module, it is possible to simplify dependencies on it from other parts of the system. This can help get rid of strong dependencies on it and use it further as a self-sufficient tool.

 public class EuropeShop : Shop { public override void CreateSale() { // 1) extracted to a repository var itemsRepository = new ItemsRepository(); var items = itemsRepository.LoadSelectedItems(); // 2) extracted to a mapper var saleItems = items.ConvertToSaleItems(); // 3) still creates a cart var cart = new Cart(); cart.Add(saleItems); // 4) all routines to apply taxes are extracted to the Tax-object new EuropeTaxes().ApplyTaxes(cart); new EuropeShopNotifier().Send(cart); // 5) extracted to a repository itemsRepository.Save(cart); } } 

As for the tests, you can get by with these scenarios. While their implementation does not interest us.

 public class EuropeTaxesTests { public void Should_not_fail_for_null() { } public void Should_apply_taxes_to_items() { } public void Should_apply_taxes_to_whole_cart() { } public void Should_apply_taxes_to_whole_cart_and_change_items() { } } public class EuropeShopNotifierTests { public void Should_not_send_when_less_or_equals_to_1000() { } public void Should_send_when_greater_than_1000() { } public void Should_raise_exception_when_cannot_send() { } } 

Rule 4: Add only tested code

As I wrote above, you should minimize changes to the old code. To do this, the old and the new / modified code can be divided. New code can be distinguished in the methods that can test the unit tests. This approach will help reduce the associated risks. There are two techniques that were described in the book “Working Effectively with the Legacy Code” (link to the book below).

Sprout method / class - this technique allows you to embed a very safe new code into the old one. The way I added the notifier is an example of this approach.

The wrap method is somewhat more complicated, but the essence is the same. Not always suitable, but only in cases when a new code is called before / after the old one. In assigning responsibilities, two calls to the ApplyTaxes method were replaced by one call. For this, it was necessary to change the second method so that the logic of the work did not break much and could be checked. This is how the class looked like before the changes.

 public class EuropeTaxes : Taxes { internal override SaleItem ApplyTaxes(Item item) { var saleItem = new SaleItem(item) { SalePrice = item.Price*1.2m }; return saleItem; } internal override void ApplyTaxes(Cart cart) { if (cart.TotalSalePrice <= 300m) return; var exclusion = 30m/cart.SaleItems.Count; foreach (var item in cart.SaleItems) if (item.SalePrice - exclusion > 100m) item.SalePrice -= exclusion; } } 

And so after. The logic of working with the elements of the basket has changed a bit, but on the whole, everything remains the same. In this case, the old method first calls the new ApplyToItems, and then its previous version. This is the essence of this technique.

 public class EuropeTaxes : Taxes { internal override void ApplyTaxes(Cart cart) { ApplyToItems(cart); ApplyToCart(cart); } private void ApplyToItems(Cart cart) { foreach (var item in cart.SaleItems) item.SalePrice = item.Price*1.2m; } private void ApplyToCart(Cart cart) { if (cart.TotalSalePrice <= 300m) return; var exclusion = 30m / cart.SaleItems.Count; foreach (var item in cart.SaleItems) if (item.SalePrice - exclusion > 100m) item.SalePrice -= exclusion; } } 

Rule 5: "Breaking" hidden dependencies

This rule is about the biggest evil in the old code: about using the new operator inside the method of one BO to create other BOs, repositories, or other complex objects. Why is that bad? The simplest explanation: it makes parts of the system strongly connected and helps to reduce their consistency. Even shorter: leads to a violation of the principle of "low coupling, high cohesion". If you look at it from the other side, then such code will be too difficult to separate into a separate, independent tool. Getting rid of such hidden dependencies at once is very time consuming. But this can be done gradually.

First, you should transfer the initialization of all dependencies to the constructor. In particular, this concerns the operators new and the creation of classes. If you have a ServiceLocator for getting instances of classes, you should also remove it into the constructor where you pull out all the necessary interfaces from it.

Secondly, the variables storing the instance of the external BO / repository must have an abstract type, and better the interface. The interface is better, because more unlocks the hands of the developer. Ultimately, this will make the module an atomic tool.

Thirdly, do not leave large methods-sheets. This is a clear indication that a method does more than what its name suggests. And then it testifies to the possible violation of SOLID, the Law of Demeter. As well as logic and Earth order.

Example

Now let's take a look at how the code that creates the cart has changed after what is above. Only the block of code that creates the basket remained unchanged. The rest is allocated to external classes and can be replaced by any implementation. Now the EuropeShop class takes on the form of an atomic tool that needs certain things that are explicitly represented in the constructor. The code becomes easier to perceive.

 public class EuropeShop : Shop { private readonly IItemsRepository _itemsRepository; private readonly Taxes.Taxes _europeTaxes; private readonly INotifier _europeShopNotifier; public EuropeShop() { _itemsRepository = new ItemsRepository(); _europeTaxes = new EuropeTaxes(); _europeShopNotifier = new EuropeShopNotifier(); } public override void CreateSale() { var items = _itemsRepository.LoadSelectedItems(); var saleItems = items.ConvertToSaleItems(); var cart = new Cart(); cart.Add(saleItems); _europeTaxes.ApplyTaxes(cart); _europeShopNotifier.Send(cart); _itemsRepository.Save(cart); } } 

Rule 6: The smaller the big tests, the better.

Large tests are various integration tests that user scripts are trying to test. Sure, they are important, but it is very expensive to check the logic of some IF in the depth of the code. As a result, only one developer, only in a special suit, surrounded by talismans, will be able to change something there. Writing such a test takes as much time, if not more, than writing the functionality itself. Supporting them is like another legacy code that is scary to change. However, these are just tests!

In order not to step on the rake of the grief of designers who are trying to test their holes with integration tests and hope that they will warn them of a possible facsape, we should separate what tests are needed for and adhere to this division. If you need an integration check, write a minimum test suite that includes positive and negative interaction scenarios. If you need to check the algorithm, write unit tests too, limiting yourself to the minimum set.

Rule 7: Do not test private methods

If suddenly you wanted to test a private method, then, apparently, you yearned for crutches. Some see nothing wrong with that. But let's look at the reasons for your Wishlist. The private method may be too complex or contain code that is not called from public methods. I am sure that any other reason that you can think of will turn out to be a characteristic of a “bad” code or design. Most likely, a part of the code from the private method should be separated into a separate method / class. Check if the first principle of SOLID is violated? This is the first reason why this is not worth doing. The second is that this way you check not the behavior of the whole module, but how it does it. The internal implementation can change regardless of the module behavior. Therefore, in this case, you get fragile tests, for the support that takes more time than necessary.

To avoid having to test private methods, present your classes as a set of atomic tools about which device you do not know anything. You expect some behavior that you are testing. This view is also true for assembly classes. Classes available to clients (from other assemblies) will be public, and those that perform internal work privately. Although, there is a difference from the methods. Internal classes can be complex, so they can be internal and also tested.

Example

For example, in order to test one condition in a private method of the EuropeTaxes class, I will not write a test for this method. I will expect that taxes will be applied in a certain way, so the test will reflect exactly that. In the test itself, I considered the handles what should happen, I took it as a standard and expect the same result from the class.

 public class EuropeTaxes : Taxes { // code skipped private void ApplyToCart(Cart cart) { if (cart.TotalSalePrice <= 300m) return; // <<< I WANT TO TEST THIS CONDIFTION var exclusion = 30m / cart.SaleItems.Count; foreach (var item in cart.SaleItems) if (item.SalePrice - exclusion > 100m) item.SalePrice -= exclusion; } } // test suite public class EuropeTaxesTests { // code skipped [Fact] public void Should_apply_taxes_to_cart_greater_300() { #region arrange // list of items which will create a cart greater 300 var saleItems = new List<Item>(new[]{new Item {Price = 83.34m}, new Item {Price = 83.34m},new Item {Price = 83.34m}}) .ConvertToSaleItems(); var cart = new Cart(); cart.Add(saleItems); const decimal expected = 83.34m*3*1.2m; #endregion // act new EuropeTaxes().ApplyTaxes(cart); // assert Assert.Equal(expected, cart.TotalSalePrice); } } 

Rule 8: Do not test algorithm methods

The name of the rule was unsuccessfully chosen here, but I haven’t yet come up with a better one. Among the “Mokists” (these are those who mock up in tests) there are those who check the number of calls to certain methods, verify the call itself, etc. In other words, it checks the internal workings of methods. This is as bad as private testing. The difference is only in the level of application of such verification. This approach again gives a lot of fragile tests, because of what TDD is not perceived by some as normal.

Rule 9: Do not modify legacy code without tests

This is the most important rule, since reflects the desire of the team to follow this path. Without the desire to move in this direction, everything that was said above does not make much sense. Since if the developer does not want to use TDD (does not understand its meaning, does not see the benefits, etc.), then its real benefits will be eroded by constant discussion of how difficult and inefficient it is.

If you are going to apply TDD, discuss in a team, add to Definition of Done and apply. At first it will be hard, as with everything new. Like any art, TDD requires constant practice, and pleasure comes as you learn. Gradually, the unit tests will be written a lot, you will begin to feel the health of your system and begin to appreciate the simplicity of writing code, describing the requirements in the first stage. There are TDD studies conducted on real large projects at Microsoft and IBM, showing a decrease in production bugs from 40% to 80%. (see link below).

Additionally

1. Book “Working Effectively with Legacy Code” by Michael Feathers
2. TDD when up to your neck in Legacy Code
3. Breaking Hidden Dependencies
4. The Legacy Code Lifecycle
5. Should you test private methods on a class?
6. Unit testing internals
7. 5 Common Misconceptions About TDD & Unit Tests
8. Intro to Unit Testing 5: Invading Legacy Code in the Name of Testability
9. Law of demeter