I've just finished reading
Michael Feathers' book
Working Effectively with Legacy Code and I highly recommend it. Despite most of it is targeting C++ and Java, there are a small number of code examples in C#, the principles most definitely still apply. This book is a must have for anyone serious about improving an existing code base without an expensive rewrite.
The biggest take-away for me was the list of strategies for breaking dependencies. This post is intended to be a quick reference index for the book. Are you working with legacy code that is not clean and tidy? You Need This Book.
The code change workflow:
- Plan the change, know what methods you need to alter.
- Understand the structure of the types and methods, diagram if necessary.
- Get the methods you want to change under test (see below for breaking dependencies). Only make the absolute minimum changes to get methods under test when no tests exist. Don't succumb to the temptation of changing too much. Basics only, break good design patterns if necessary; get the methods under test then fix the design breaks.
- If possible program with a partner, and they should be constantly asking "what are you doing now? One thing at a time, pick one. Are you sure we need that to get the method under test?" Change chairs every 20-30 minutes.
- Make the original intended change.
Adapt ParameterPage 326. Use this when you can't extract a full interface or its impractical to do so. This works when the usage of the method parameter only accesses a small number of members on the parameter. Ideally the return types of the members used on the parameters must be simple or mockable types.
public void Foo(HttpWebRequest request) {
...
var type = request.ContentType;
...
}
becomes
public void Foo(string contentType)
{
...
}
or
public void Foo(IRequestMetaData requestData)
{
...
var type = requestData.ContentType;
...
}
Break Out Method Object
Page 330. Useful when the method you want to change is very large, or the work required to get the whole method under test is significant. Also consider Expose Static Method. The idea is to move the entire method to a new class and the parameters of the method become instance fields, and properties and/or constructor arguments. Once in its own class it should be easier to break dependencies using the other techniques.
Encapsulate Global References
Page 339. This is used to break dependencies on static / global references. Essentially you extract the global data encapsulate it in an interfaced (or overridable) class. It might be appropriate to Expose a Static Setter or access the new encapsulated class via a singleton
IoC container.
Expose Static Method
Page 345. Use this when a method you are trying to get into test doesn't access any instance members. Useful when the class itself has many dependencies. Making the method static means you can concentrate on the method and not the other dependencies in the class.
Resharper will suggest making a method static by default.
Extract and Override Call
Page 348. This is useful when a target method depends on another within the same class which you are not really concerned with right now. Remove the dependencies on the other method by creating a test class and inheriting from the class and overriding the method you are not concerned with. This way you can call the original method and your overriden method can return dummy data and skip other dependencies.
[Edit - also see comments below. Another option would of course be to use an IoC container to avoid using virtual method calls in the constructor. However sometimes introducing an IoC container into legacy code can be time consuming; although it is preferential when possible.]
Extract and Override Factory Method
Page 350. When a class you need to change creates other dependencies in the constructor it can be useful to extract those creation lines of code elsewhere. Move all the code that creates other types into a new virtual method and call it from the constructor. A test class can be used to then override the new virtual
factory method to return mock dependencies.
Extract and Override Getter
Page 352. This is again concerned with creation of dependencies in the constructor or field initialisers. Sometimes a constructor can contain a large number of arguments or the class accesses a high number of singletons. Also when the constructor creates types sometimes using Extract and Override Factory Method isn't practical if it means creating many
factory methods. This technique creates properties that return the types, the properties refer to private fields and are initialised to null. All code within the class is refactored to use the properties. On first access the property initialises the field and returns the field thereafter. Under test private accessors (MsTest) or
reflection can be used to set the field.
Extract Interface
Page 362. I see this as the primary tool for breaking dependencies. The idea is to take a dependency of a class and create an interface that matches its public API. Ideally you control the source of the dependency class and you implement the new interface with that class. (If you don't read the chapter on "My Application is All API calls" Chapter 15). The next step is to refactor the code you are looking at changing to use the new interface.
Introduce Instance Delegator
Page 369. This is another technique for working with Static classes. The idea is to have an instance method that delegates to the static method. All dependent code uses the instance methods NOT the static methods. (Changes can be piece-meal, only change code that your are trying to get under test right now). Now a testing subclass (mock) can be created overriding the behaviour.
Introduce Static Setter
Page 372. Although mutable static data should be avoided working with it is almost inevitable. Use this strategy when you have static global data you need to set or change the behaviour under test. The idea is to have a setter that you use to swap out static data under test. Fortunately in C# you can use a private accessor to set a private property or field and avoid having a public setter that could be misused. Another way is to use an internal setter and use the InternalsVisibleToAttribute.
Parameterise Constructor
Page 379. This is another way to remove code from a constructor that instantiates other classes. Simply create constructor parameters for any class created in the constructor and make the calling class instantiate it and pass it in.
Parameterise Method
Page 383. This is very similar to Parameterize Constructor, although there is good reasons to not create classes in constructors there's no broad rule to not do this in methods, because that would be kind of pointless. The idea is to pass in any difficult to create dependencies in as parameters. This is a bit of a cop out, as it just pushes the problem into another class that you might need to write tests for tomorrow. First try Extract Interface and use
DI, or Extract Factory Method, or Extract and Override Call.
Primitivise Parameter
Page 385. This is a really simple handy thing to try first when working with difficult method parameters. Resharper has really good support for this. The idea is to find the simplest base class for a parameter and use that instead. Quite often there is an existing interface or abstract class that allows the method to still function and is easier to mock.
Pull Up Feature
Page 388. If you are prevented from getting a class under test because some unrelated methods in it have difficult and also unrelated dependencies, you might be able to use this technique. Create an abstract base class pull your affected methods into the base class, and work through compile errors until it compiles. You might also need to use Extract Interface or other techniques as well. Then you can create a testing subclass that tests only the affected methods.
Push Down Dependency
Page 392. When Subclass and Override Method is problematic because there are too many dependencies, this is another option. You might be able to make the current class abstract and push the difficult dependencies down into a subclass. You may need to use this in conjunction with Extract Interface for the dependencies or you might be able to wrap them into functional calls rather than using the dependency directly. After you're done with this refactoring you will be able to create a testing subclass to test your affected methods.
Replace Global Reference with Getter
Page 399. This is another technique for working around global data. This technique only really works in C# if the global data is not static (ie returned from a static or global container). This technique requires you to replace all public fields or properties on the global instance with virtual property getters. This allows you to subclass the global with a fake under test. I personally find this a bit awkward and would rather use Extract Interface and maybe a IoC container, or introduce a Static Setter.
Subclass and Override Method
Page 401. This is a core technique for focusing on the methods you care about using inheritance to override the ones you don't. Create a subclass of the subject class and override methods you don't care about that are called from the one you are trying to get under test. You will need to make the members affected virtual and unseal the class if necessary. This is a good starting out technique when working on a class that has never been unit tested before.
Replace Dependency with a Delegate
Page 396 - this is an adaptation of Replace Function with Function Pointer. Sometimes it might be simpler than using Extract Interface or other techniques which can require a bit of work. The idea is to pass in a delegate instead of the difficult dependency itself. This only works if the usage of the dependency involves simple or already testable return types.
public void Write(Stream stream) {
...
stream.Write(bytes);
}
becomes
public void Write(Action<byte[]> writer) {
...
writer(bytes);
}
Lazyload Dependency Property Getters
This is something I do frequently to avoid creating or getting dependencies in the constructor. Its essentially an adaptation of Extract Factory Method. The idea is to use properties within a class to access all external dependencies. By default on first access the dependency will return the production default class, but using a private accessor you can inject a fake in test.
public Wheel TheWheel {
get {
return this.backingField ?? (this.backingField = new ProductionWheel());
}
}
// TODO: Code examples
