A Refactoring Workout: Relentlessly Green

We don’t practice much—not the way athletes and musicians do.

Of course, they have an advantage over us, since they have thousands of exercises and progressions that they can use to improve any aspect of their craft.

We, on the other hand, are making it up as we go along.

Refactoring is the process of changing a software system in such a way that it does not alter the external behavior of the code yet improves its internal structure. —Martin Fowler

How do you get better at refactoring?

Well, you read the refactoring books and do more refactoring. Basically.

That’s not quite as helpful as, say “practice this independence drill using a metronome until you can play it faultlessly for 120 seconds. Start at 30 beats per minute (BPM) and work the drill in increments of 5 BPM until you can consistently play it at 180 beats per minute.”.

Notice the word drill in there. A drill is a very focused form of practice. It targets a technique to the point of warping it, making it possible to repeat a particular aspect of the technique over and over, notifying you immediately if you’re doing it wrong.

The immediate feedback is crucial in a drill, because if you’re practicing incorrectly, the repetition will solidify the flaws making it even more difficult to perform correctly later.

Practice doesn’t make perfect, practice makes permanent. —Larry Gelwix

Refactoring is Not Rehacktoring

Too often we take leaps, not steps, while refactoring. However, while we’re hacking away at something which we just know in our gut is going to work (eventually), the tests are failing.

That is, if we’re even running them.

One of the challenges of refactoring is succession—how to slice the work of a refactoring into safe steps and how to order those steps. —Kent Beck

A safe step is one which does not cause the tests to fail.

This refactoring drill focuses on succession, repeating the act of choosing a tiny, safe step over and over again, with an automated test providing continuous and immediate feedback.

Refactoring Under Green

The starting point of the exercise is a simple test with a trivial implementation that is passing. The goal is to refactor the code in tiny, safe steps until it has become a generalized solution.

The size of a step is limited by what your editor can revert with a single undo.

There are two rules:

• Run the test after every single change.
• If the test fails, it should take exactly one undo to get back to green.

The test suite is minimal, asserting that the default string representation of a Checkerboard instance returns a grid with alternating black (B) and white (W) squares. The board in question is as small as possible, while still defining a checkered pattern.

gem 'minitest', '~> 5.3'
require 'minitest/autorun'
require_relative 'checkerboard'

class CheckerboardTest < Minitest::Test
  def test_small_board
    expected = <<-BOARD
B W
W B
BOARD
    assert_equal expected, Checkerboard.new(2).to_s
  end
end

The implementation hard-codes the string representation of the 2×2 grid.

class Checkerboard
  def initialize(_)
  end

  def to_s
    "B W\nW B\n"
  end
end

When the refactoring is complete, it should be possible to call Checkerboard.new with any size and get a properly formatted checkerboard.

It’s tempting to add another failing test at this point, perhaps for a 3×3 board, in order to triangulate towards better design. But this isn’t an exercise in Test-Driven Development.

In this drill, the design will be driven by conscious choices about the next safe step, not by a failing test. If the step turns out not to be safe, hit undo once and you should be back to green.

Where to Begin?

The current algorithm has some issues.

def to_s
  "B W\nW B\n"
end

For one, it doesn’t scale. Also, it mixes data and presentation.

Ideally, the grid would be built up separately, and the to_s method would manipulate the grid to provide a string representation. The newlines make it clear that there are two of something here. Two rows. That smells like an array.

How do you go from a string to an array without failing somewhere along the way?

How do you disconnect a hospital from the electrical grid without killing a few patients?

Redundancy.

Put something in place that will let you fail-over safely.

The Redundancy Ploy

Write a completely new implementation, and stick it right before the old implementation.

def to_s
  ["B W\n", "W B\n"].join
  "B W\nW B\n"
end

Run the test.

The new implementation hasn’t been vetted yet, but it has been executed, which provides a syntax check.

Delete the original implementation, and run the test again.

def to_s
  ["B W\n", "W B\n"].join
end

If it had failed, a single undo would put the working code back.

Use the same technique to move the newline out of the hard-coded rows.

def to_s
  ["B W", "W B"].map {|row| row + "\n"}.join
end

We’re so used to thinking of duplication as the enemy. However, in refactoring, duplication is a short-term, low-cost investment that pays excellent returns.

The Setup-and-Swap Ploy

It’s difficult to get at the individual cells. It would be easier if they were in an array of their own. The setup-and-swap ploy adds all the code necessary to be able to make the final change in one small step.

def to_s
  rows = []
  rows << "B W"
  rows << "W B"
  ["B W", "W B"].map {|row| row + "\n"}.join
end

This adds three lines of code that don’t have any bearing on the state of the test suite, but they make it very easy to replace the hard-coded array with the new rows variable.

def to_s
  rows = []
  rows << "B W"
  rows << "W B"
  rows.map {|row| row + "\n"}.join
end

Each row itself is still hard-coded, but can be transformed independently using another setup-and-swap.

row = ["B", "W"].join(" ")
rows << "B W"

row = ["B", "W"].join(" ")
rows << row

def to_s
  rows = []
  row = ["B", "W"].join(" ")
  rows << row
  row = ["W", "B"].join(" ")
  rows << row
  rows.map {|row| row + "\n"}.join
end

This has moved the whitespace away from the individual cells, but we can do better. The join belongs within the loop.

This is particularly tricky, since we have two joins that need to be collapsed into a single spot. If we move one, the the test will fail. If we delete both, the test will fail. If we stick a join on the row within the loop first, the test will fail.

We could use the redundancy ploy, introducing a rows2 variable, but there’s an easier way.

The Null Method Ploy

There’s no reason why String can’t have a join method!

class String
  def join(_)
    self
  end
end

class Checkerboard
  # ...
end

This makes it trivial to make the change without breaking anything.

def to_s
  rows = []
  row = ["B", "W"].join(" ")
  rows << row
  row = ["W", "B"].join(" ")
  rows << row
  rows.map {|row| row.join(" ") + "\n"}.join
end

Now the row within the loop can handle both arrays and strings, and we can delete the original joins, along with the null method.

def to_s
  rows = []
  row = ["B", "W"]
  rows << row
  row = ["W", "B"]
  rows << row
  rows.map {|row| row.join(" ") + "\n"}.join
end

The two row assignments are similar, but not identical. Use the redundancy ploy to separate the parts that vary from the parts that stay the same.

def to_s
  rows = []

  row = []
  row << "B"
  row << "W"
  rows << row

  row = []
  row << "W"
  row << "B"
  rows << row

  rows.map {|row| row.join(" ") + "\n"}.join
end

This solution will only scale if we introduce a loop. Well, two actually.

The Wind-Unwind Ploy

There are two chunks that have the same setup and the same finish. Call these Chunk A and Chunk B.

# chunk a

# chunk b

The wind-unwind ploy uses a loop to wind the chunks up, and a conditional to unwind them back to where they were.

2.times do
  if condition A
    # chunk a
  else
    # chunk b
  end
end

Then common code can be moved out of the conditional:

2.times do
  # common code
  if condition A
    # variation a
  else
    # variation b
  end
  # common code
end

Combine the redundancy ploy with the wind-unwind ploy to introduce the loop safely.

def to_s
  rows = []
  2.times {|y|
    row = []
    if y == 0
      row << "B"
      row << "W"
    else
      row << "W"
      row << "B"
    end
    rows << row
  }
  rows.map {|row| row.join(" ") + "\n"}.join
end

There’s still a lot of similarity in shoveling cells into the row. The only difference is order. Apply the wind-unwind ploy again.

def to_s
  rows = []
  2.times {|y|
    row = []
    2.times {|x|
      if y == 0
        if x == 0
          row << "B"
        else
          row << "W"
        end
      else
        if x == 0
          row << "W"
        else
          row << "B"
        end
      end
    }
    rows << row
  }

  rows.map {|row| row.join(" ") + "\n"}.join
end

This is, admittedly, pretty terrible, but the tests are passing and the nested conditionals can be fixed using the redundancy ploy.

def to_s
  rows = []
  2.times {|y|
    row = []
    2.times {|x|
      if x == y
        row << "B"
      else
        row << "W"
      end
    }
    rows << row
  }
  rows.map {|row| row.join(" ") + "\n"}.join
end

x == y is only really valid for the 2×2 checkerboard. In a larger checkerboard, this produces a diagonal stripe.

There are a couple of valid approaches. First:

if (x.even? && y.even?) || (x.odd? && y.odd?)
  # it's black
else
  # it's white
end

The other is more succinct:

if (x+y).even?
  # it's black
else
  # it's white
end

This algorithm will work for a checkerboard of any size, provided that we loop enough times. We’ve been passing the argument that we need to the new Checkerboard instance all along. Use the setup-and-swap ploy to make that data available to the rest of the instance, and then replace the magic numbers with calls to size.

attr_reader :size
def initialize(size)
  @size = size
end

def to_s
  rows = []
  size.times {|y|
    row = []
    size.times {|x|
      if (x+y).even?
        row << "B"
      else
        row << "W"
      end
    }
    rows << row
  }
  rows.map {|row| row.join(" ") + "\n"}.join
end

Sanity test the solution by adding a second test that proves that it works.

def test_chess_board
    expected = <<-BOARD
B W B W B W B W
W B W B W B W B
B W B W B W B W
W B W B W B W B
B W B W B W B W
W B W B W B W B
B W B W B W B W
W B W B W B W B
BOARD
    assert_equal expected, Checkerboard.new(8).to_s
  end

This is not a final solution. The to_s method is afflicted with a number of code smells. Perhaps there should be a Cell object which has a to_s method that decides what the string representation of black or white looks like. All of these changes can be made using safe steps.

Closing Thoughts

Wait. Really? You would monkey-patch String just so that you can make a change in one step rather than two?

Uh, no. Not really. That would be ridiculous.

These ploys are hyper-focused exaggerations. Variations on them can be useful in the real world, but the biggest outcome of performing this drill a few times is a shift in perspective.

Suddenly you will find yourself finding sneaky, creative ways of making a change without having to take the site down or having to deal with long-lived branches.

You may also find that your sense of scale gets adjusted. Small steps tend to be smaller than we think.

And lastly, you might discover that your tolerance for grotesque code increases.

After all, you can always refactor.