You will probably want to read my conceptual post on this topic before this one.
The kata that I’m using can be found at github here. My walkthrough is in the EricGuSolution branch, and I checked in whenever I hit a good stopping point. When you see something like:
Commit: Added RecipeManager class
you can find that commit on the branch and look at the change that I made. The checkin history is fairly coarse; if you want a more atomic view, go over to the original version of the kata, and there you’ll find pretty much a per-change view of the transformations.
Our patient
We start with a very simple Windows Forms application for managing recipes. It allows users to create/edit/delete recipes, and the user can also decide where to store their recipes. The goal is to add unit tests for it. The code is pretty tiny, but it’s pretty convoluted; there is UI code tied in with file system code, and it’s not at all clear how we can get tested.
I’m going to be doing TDD as much as possible here, so the first thing to do is to dive right in and start writing tests, right?
The answer to that is “nope”. Okay, if you are trying to add functionality, you can use the techniques in Feather’s excellent book, “Working Effectively with Legacy Code”, but let’s just pretend we’ve done that and are unhappy with the result, so we’re going to refactor to make it easier to test.
The first thing that I want you to do is to look at the application & code, and find all the ports, and then write down a general description of what each port does. A port is something that a program uses to interface with an external dependency. Go do that, write them down, and then code back.
The Ports
I identified three ports in the system:
- A port that loads/saves/lists/deletes recipes
- A port that loads/saves the location of the recipes
- A port that handles all the interactions with the user (ie “UI”)
You could conceivably break some of these up; perhaps the UI port that deals with recipes is different than the one that deals with the recipe storage directory. We’ll see what happens there later on.
If you wanted, you could go to the next level of detail and write out the details of the interface of each port, but I find it easier to pull that out of the code as I work.
How do I do this without breaking things?
That’s a really good question. There are a number of techniques that will reduce the chance of that happening:
- If your language has a refactoring tool available, use it. This will drastically reduce the number of bugs that you introduce. I’m working in C#, so I’m going to be using Resharper.
- Run existing tests (integrated tests, other automated tests, manual tests) to verify that things still work.
- Write pinning tests around the code you are going to change.
- Work in small chunks, and test often.
- Be very careful. My favorite method of being very careful is to pair with somebody, and I would prefer to do it even if I have pretty good tests.
Wherever possible, I used resharper to do the transformations.
Create an adapter
An adapter is an implementation of a port. I’m going to do the recipe one first. My goal here is to take all the code that deals with these operations and get it in one place. Reading through the code in Form1.cs, I see that there the LoadRecipes() method. That seems like something our port should be able to do. It has the following code:
private void LoadRecipes()
{
string directory = GetRecipeDirectory();
DirectoryInfo directoryInfo = new DirectoryInfo(directory);
directoryInfo.Create();
m_recipes = directoryInfo.GetFiles("*")
.Select(fileInfo => new Recipe { Name = fileInfo.Name, Size = fileInfo.Length, Text = File.ReadAllText(fileInfo.FullName) }).ToList();
PopulateList();
}
I see three things going on here. First, we get a string from another method, then we do some of our processing, then we call the “PopulateList()” method. The first and the last thing don’t really have anything to do with the concept of dealing with recipes, so I’ll extract the middle part out into a separate method (named “LoadRecipesPort()” because I couldn’t come up with a better name for it).
private void LoadRecipes()
{
string directory = GetRecipeDirectory();
m_recipes = LoadRecipesPort(directory);
PopulateList();
}
private static List<Recipe> LoadRecipesPort(string directory)
{
DirectoryInfo directoryInfo = new DirectoryInfo(directory);
directoryInfo.Create();
return directoryInfo.GetFiles("*")
.Select(
fileInfo =>
new Recipe
{
Name = fileInfo.Name,
Size = fileInfo.Length,
Text = File.ReadAllText(fileInfo.FullName)
})
.ToList();
}
Note that the extracted method is static; that verifies that it doesn’t have any dependencies on anything in the class.
I read down some more, and come across the code for deleting recipes:
private void DeleteClick(object sender, EventArgs e)
{
foreach (RecipeListViewItem recipeListViewItem in listView1.SelectedItems)
{
m_recipes.Remove(recipeListViewItem.Recipe);
string directory = GetRecipeDirectory();
File.Delete(directory + @"\" + recipeListViewItem.Recipe.Name);
}
PopulateList();
NewClick(null, null);
}
There is only one line there – the call to File.Delete(). I pull that out into a separate method:
private static void DeleteRecipe(string directory, string name)
{
File.Delete(directory + @"\" + name);
}
Next is the code to save the recipe. I extract that out:
private static void SaveRecipe(string directory, string name, string directions)
{
File.WriteAllText(Path.Combine(directory, name), directions);
}
That is all of the code that deals with recipes.
Commit: Extracted recipe code into static methods
<aside>
You may have noticed that there is other code in the program that deals with the file system, but I did not extract it. That is very deliberate; my goal is to extract out the implementation of a specific port. Similarly, if I had been using a database rather than a file system, I would extract only the database code that dealt with recipes.
This is how this pattern differs from a more traditional “wrapper” approach, and is hugely important, as I hope you will soon see.
</aside>
The adapter is born
I do an “extract class” refactoring and pull out the three methods into a RecipeStore class. I convert all three of them to instance methods with resharper refactorings (add a parameter of type RecipeStore to each of them, then make them non-static, plus a bit of hand-editing in the form class). I also take the directory parameter and push it into the constructor. That cleans up the code quite a bit, and I end up with the following class:
public class RecipeStore
{
private string m_directory;
public RecipeStore(string directory)
{
m_directory = directory;
}
public List<Recipe> Load()
{
DirectoryInfo directoryInfo = new DirectoryInfo(m_directory);
directoryInfo.Create();
return directoryInfo.GetFiles("*")
.Select(
fileInfo =>
new Recipe
{
Name = fileInfo.Name,
Size = fileInfo.Length,
Text = File.ReadAllText(fileInfo.FullName)
})
.ToList();
}
public void Delete(string name)
{
File.Delete(m_directory + @"\" + name);
}
public void Save(string name, string directions)
{
File.WriteAllText(Path.Combine(m_directory, name), directions);
}
}
Commit: RecipeStore instance class with directory in constructor
Take a look at the class, and evaluate it from a design perspective. I’m pretty happy with it; it does only one thing, and the fact that it’s storing recipes in a file system isn’t apparent from the method signature. The form code looks better as well.
Extract the port interface & write a simulator
I now have the adapter, so I can extract out the defining IRecipeStore interface.
public interface IRecipeStore
{
List<Recipe> Load();
void Delete(string name);
void Save(string name, string directions);
}
I’ll add a new adapter class that implements this interface:
class RecipeStoreSimulator: IRecipeStore
{
public List<Recipe> Load()
{
throw new NotImplementedException();
}
public void Delete(string name)
{
throw new NotImplementedException();
}
public void Save(string name, string directions)
{
throw new NotImplementedException();
}
}
The simulator is going to be an in-memory implementation of the recipe store, which which will make it very good for unit tests. Since it’s going to be in-memory, it doesn’t have any dependencies and therefore I can write unit tests for it. I’ll do that with TDD.
Commit: RecipeStoreSimulator with tests
It was a very simple interface, so it only took me about 15 minutes to write it. It’s not terribly robust, however; it has no error-handling at all. I now have a simulator that I can use to test any code that uses the RecipeStore abstraction. But wait a second; the tests I wrote for the simulator are really tests for the port.
If I slightly modify my tests so that they use an IRecipeStore, I can re-purpose them to work with any implementation of that port. I do that, but I start seeing failures, because the tests assume an empty recipe store. If I change the tests to clean up after themselves, it should help…
Once I’ve done that, I can successfully run the port unit tests against the filesystem recipestore.
Commit: Unit tests set up to test RecipeStore
RecipeStoreLocator
We’ll now repeat the same pattern, this time with the code that figures out where the RecipeStore is located. I make the methods static, push them into a separate class, and turn them back into instance methods.
When I first looked at the code, I was tempted not to do this port, because the code is very specific to finding a directory, and the RecipeStore is the only thing that uses it, so I could have just put the code in the RecipeStore. After a bit of thought, I decided that “where do I store my recipes” is a separate abstraction, and therefore having a locator was a good idea.
Commit: RecipeStoreLocator class added
I create the Simulator and unit tests, but when I go to run them, I find that I’m missing something; the abstraction has no way to reset itself to the initial state because the file persists on disk. I add a ResetToDefault() method, and then it works fine.
Commit: Finished RecipeStoreLocator + simulator + unit tests
Status check & on to the UI
Let’s take a minute and see what we’ve accomplished. We’ve created two new port abstractions and pulled some messy code out of the form class, but we haven’t gotten much closer to be able to test the code in the form class itself. For example, when we call LoadRecipes(), we should get the recipes from the store, and then push them out into the UI. How can we test that code?
Let’s try the same sort of transformations on the UI dependency. We’ll start with PopulateList():
private void PopulateList()
{
listView1.Items.Clear();
foreach (Recipe recipe in m_recipes)
{
listView1.Items.Add(new RecipeListViewItem(recipe));
}
}
The first change is to make this into a static method. That will require me to pass the listview and the recipe list as parameters:
private static void PopulateList(ListView listView, List<Recipe> recipes)
{
listView.Items.Clear();
foreach (Recipe recipe in recipes)
{
listView.Items.Add(new RecipeListViewItem(recipe));
}
}
And I’ll pull it out into a new class:
public class RecipeManagerUI
{
private ListView m_listView;
public RecipeManagerUI(ListView listView)
{
m_listView = listView;
}
public void PopulateList(List<Recipe> recipes)
{
m_listView.Items.Clear();
foreach (Recipe recipe in recipes)
{
m_listView.Items.Add(new RecipeListViewItem(recipe));
}
}
}
This leaves the following implementation for LoadRecipes():
private void LoadRecipes()
{
m_recipes = m_recipeStore.Load();
m_recipeManagerUI.PopulateList(m_recipes);
}
That looks like a testable bit of code; it calls load and then calls PopulateList with the result. I extract it into a RecipeManager class (not sure about that name right now), make it an instance method, add a constructor to take the recipe store and ui instances, and pull the list of recipes into this class as well. I end up with the following:
public class RecipeManager
{
private RecipeStore m_recipeStore;
private RecipeManagerUI m_recipeManagerUi;
private List<Recipe> m_recipes;
public RecipeManager(RecipeStore recipeStore, RecipeManagerUI recipeManagerUI)
{
m_recipeManagerUi = recipeManagerUI;
m_recipeStore = recipeStore;
}
public List<Recipe> Recipes { get { return m_recipes; } }
public void LoadRecipes()
{
m_recipes = m_recipeStore.Load();
m_recipeManagerUi.PopulateList(m_recipes);
}
}
Commit: Added RecipeManager class
Now to test LoadRecipes, I want to write:
[TestMethod()]
public void when_I_call_LoadRecipes_with_two_recipes_in_the_store__it_sends_them_to_the_UI_class()
{
RecipeStoreSimulator recipeStore = new RecipeStoreSimulator();
recipeStore.Save("Grits", "Stir");
recipeStore.Save("Bacon", "Fry");
RecipeManagerUISimulator recipeManagerUI = new RecipeManagerUISimulator();
RecipeManager recipeManager = new RecipeManager(recipeStore, new RecipeManagerUISimulator());
recipeManager.LoadRecipes();
Assert.AreEqual(2, RecipeManagerUI.Recipes.Count);
RecipeStoreSimulatorTests.ValidateRecipe(recipeManagerUI.Recipes, 0, "Grits", "Stir");
RecipeStoreSimulatorTests.ValidateRecipe(recipeManagerUI.Recipes, 1, "Bacon", "Fry");
}
I don’t have the appropriate UI simulator, so I’ll extract the interface and write the simulator, including some unit tests.
Commit: First full test in RecipeManager
In the tests, I need to verify that RecipeManager.LoadRecipes() passes the recipes off to the UI, which means the simulator needs to support a property that isn’t needed by the new class. I try to avoid these whenever possible, but when I have to use them, I name them to be clear that they are something outside of the port interface. In this case, I called it SimulatorRecipes.
We now have a bit of logic that was untested in the form class in a new class that is tested.
UI Events
Looking at the rest of the methods in the form class, they all happen when the user does something. That means we’re going to have to get a bit more complicated. The basic pattern is that we will put an event on our UI port, and it will either hook to the actual event in the real UI class, or to a SimulateClick() method in the simulator.
Let’s start with the simplest one. NewClick() looks like this:
private void NewClick(object sender, EventArgs e)
{
textBoxName.Text = "";
textBoxObjectData.Text = "";
}
To move this into the RecipeManager class, I’ll need to add abstractions to the UI class for the click and for the two textbox values.
I start by pulling all of the UI event hookup code out of the InitializeComponent() method and into the Form1 constructor. Then, I added a NewClick event to the UI port interface and both adapters that implement the interface. It now looks like this:
public interface IRecipeManagerUI
{
void PopulateList(List<Recipe> recipes);
event Action NewClick;
string RecipeName { get; set; }
string RecipeDirections { get; set; }
}
And, I’ll go off and implement these in the UI class, the simulator class, and the simulator test class.
<aside>
I’m not sure that NewClick is the best name for the event, because “click” seems bound to the UI paradigm. Perhaps NewRecipe would be a better name…
</aside>
Commit: Fixed code to test clicking the new button
Note that I didn’t write tests for the simulator code in this case. Because of the nature of the UI class, I can’t run tests across the two implementations to make sure they are the same (I could maybe do so if I did some other kind of verification, but I’m not sure it’s worth it). This code mostly fits in the “if it works at all, it’s going to work” category, so I don’t feel that strongly about testing it.
The test ends up looking like this:
[TestMethod()]
public void when_I_click_on_new__it_clears_the_name_and_directions()
{
RecipeManagerUISimulator recipeManagerUI = new RecipeManagerUISimulator();
RecipeManager recipeManager = new RecipeManager(null, recipeManagerUI);
recipeManagerUI.RecipeName = "Grits";
recipeManagerUI.RecipeDirections = "Stir";
Assert.AreEqual("Grits", recipeManagerUI.RecipeName);
Assert.AreEqual("Stir", recipeManagerUI.RecipeDirections);
recipeManagerUI.SimulateNewClick();
Assert.AreEqual("", recipeManagerUI.RecipeName);
Assert.AreEqual("", recipeManagerUI.RecipeDirections);
}
That works. We’ll keep going with the same approach – choose an event handler, and go from there. We’re going to do SaveClick() this time:
private void SaveClick(object sender, EventArgs e)
{
m_recipeStore.Save(textBoxName.Text, textBoxObjectData.Text);
m_recipeManager.LoadRecipes();
}
We’ll try writing the test first:
[TestMethod()]
public void when_I_click_on_save__it_stores_the_recipe_to_the_store_and_updates_the_display()
{
RecipeStoreSimulator recipeStore = new RecipeStoreSimulator();
RecipeManagerUISimulator recipeManagerUI = new RecipeManagerUISimulator();
RecipeManager recipeManager = new RecipeManager(recipeStore, recipeManagerUI);
recipeManagerUI.RecipeName = "Grits";
recipeManagerUI.RecipeDirections = "Stir";
recipeManagerUI.SimulateSaveClick();
var recipes = recipeStore.Load();
RecipeStoreSimulatorTests.ValidateRecipe(recipes, 0, "Grits", "Stir");
recipes = recipeManagerUI.SimulatorRecipes;
RecipeStoreSimulatorTests.ValidateRecipe(recipes, 0, "Grits", "Stir");
}
That was simple; all I had to do was stub out the SimulateSaveClick() method. The test fails, of course. About 10 minutes of work, and it passes, and the real UI works as well.
Commit: Added Save
Commit: Added in Selecting an item in the UI
Commit: Added support for deleting recipes
To be able to support changing the recipe directory required the recipe store to understand that concept. This was done by adding a new RecipeDirectory property, and implementing it in both IRecipeStore adapters.
Commit: Added support to change recipe store directory
All done
Let’s look at what is left in the form class:
public partial class Form1 : Form
{
private RecipeManager m_recipeManager;
public Form1()
{
InitializeComponent();
var recipeManagerUI = new RecipeManagerUI(listView1,
buttonNew,
buttonSave,
buttonDelete,
buttonSaveRecipeDirectory,
textBoxName,
textBoxObjectData,
textBoxRecipeDirectory);
var recipeStoreLocator = new RecipeStoreLocator();
var recipeStore = new RecipeStore(recipeStoreLocator.GetRecipeDirectory());
m_recipeManager = new RecipeManager(recipeStore, recipeStoreLocator, recipeManagerUI);
m_recipeManager.Initialize();
}
}
This is the entirety of the form class; it just creates the RecipeManagerUI class (which encapsulates everything related to the UI), the RecipeStoreLocator class, the RecipeStore class, and finally, the RecipeManager class. It then calls Initialize() on the manager, and, at that point, it’s up and running.
Looking through the code, I did a little cleanup:
- I renamed RecipeDirectory to RecipeLocation, because that’s a more abstract description.
- I renamed Recipe.Text to Recipe.Directions, because it has been buggin’ me…
- Added in testing for Recipe.Size
Commit: Cleanup
Recent Comments