Jump to section

Related posts

Refactoring Part 3: More Testing

In Part 2 we started adding unit tests and refactoring for our new feature.  Now we’ll finish up the rest of the changes and then move on to separating out the UI code so we can generate the appropriate number of boxes to represent doors.  As a refresher, the changes we need to make are:

  • switchDoor and stay both need refactoring to eliminate duplicate behavior and hard coding
  • resetAll needs to operate on more than 3 doors
  • The UI is dependent on 3 doors

switchDoor

switchDoor has the same issue as determineContents did in our last part: it operates on the global game object.  Once again we’ll rely on our seam concept and pass in the game object from all the call sites.  Unfortunately since the only call site is in the UI, we’ll have to play a game manually to verify our changes are ok.

9187734

Then we’ll add characterization tests in order to verify the existing behavior.  We’ll also pass game into updateScore since it’s a dependency of switchDoor and we don’t want to screw up global state in our unit tests.

7cb8e5d

And finally we will add a new failing test for the updated behavior then implement that required behavior.

d15f0e8

Mechanically, this process is identical to the changes we made in determineContents which makes it easy to verify that the changes we made are correct.

Refactoring out duplicate behavior

Looking at the code in switchDoor and stay, we can see only two things that vary:

  • We need to actually switch the door in switchDoor
  • We need to increment different fields in each method

Other than that, the methods are actually the same.  First, lets make them as similar as possible. stay does not take a local game object so lets change that and ensure the change is safe by making it in isolation

a74a122

Next, lets refactor out the changing of game.selected in each method.  In stay we doNothing so we can just call a no-op method.  In switchDoor we need to actuallySwitch.  The purpose of this refactoring is to illustrate to ourselves that there is a Strategy which determines how we should switch the doors that we’ve picked.

4f9ea9a

Now the only difference is the field in the state that we’re going to change.  Lets hard code that at the top of each method to make it easier to extract later.

adf045e

Finally, we can combine the methods (under a better name) and pass in the proper arguments for each one.  Running our characterization tests shows that the refactoring is successful

bc30d29

That refactoring was actually quite involved but the important point to consider is how we performed it.  We didn’t focus immediately on what was the same and try to extract it, we focused on what was different first.  Only after we had extracted every difference from the two methods did we try to combine them into one.  Focusing on extracting differences in this way gives you a greater chance of completing your refactor in a non-breaking fashion because the changes are smaller and you always have an easy path backwards.

Notably, we can now add unit tests to actuallySwitched in isolation of the field incrementing behavior.  Since our code is properly factored, everything becomes easier to test and we can write smaller test cases in the future.

Continuing

Now that we have most of our controller code properly factored, we can take a look at factoring out our UI code and then adding the new functionality.