George Dinwiddie's blog

Jeff Langr requested a Blog Pair for a tic-tac-toe program that he says “screams out for refactoring.” I looked at the code, and didn’t see an easy way to refactor it given the fragment posted. I did, however see a different solution.

To me, the code screamed out for a table-driven solution rather than an algorithmic one. Rather than have four different methods for detecting a winner, I built one. Here’s my solution:

  boolean isWinner(Player currentPlayer) {
    Position[][] winningCombinations = {
        {new Position(0,0), new Position(0,1), new Position(0,2)},
        {new Position(1,0), new Position(1,1), new Position(1,2)},
        {new Position(2,0), new Position(2,1), new Position(2,2)},
        {new Position(0,0), new Position(1,0), new Position(2,0)},
        {new Position(0,1), new Position(1,1), new Position(2,1)},
        {new Position(0,2), new Position(1,2), new Position(2,2)},
        {new Position(0,0), new Position(1,1), new Position(2,2)},
        {new Position(0,2), new Position(1,1), new Position(2,0)}
        };
    for (int i = 0; i < winningCombinations.length; i++) {
      Position[] aCombination = winningCombinations[i];
      boolean winner = true;
      for (int j = 0; j < aCombination.length; j++) {
        Position aPosition = aCombination[j];
        if (!currentPlayer.equals(grid[aPosition.x][aPosition.y])) {
          winner = false;
          break;
        }
      }
      if (winner) return winner;
    }
    return false;
  }

This solution reminds me of C code I’ve written. To see the tests and the steps along the way, browse the code in Subversion. You’ll also see a stupid mistake I made by rushing (it’s late) and not following my normal coding standards (“Always put a conditional clause in curly braces”). Fortunately, a test gave a surprising result and I found the problem.

In retrospect, I could have refactored to this from Jeff’s code. The key would have been to wrap the four “is…Winner()” methods with one, and then use the sort of “strangler refactor” that I used on my own code to get from conditionals to table-driven decision-making.