Reusable code through separation of concerns

10 Oct 2014

We should aim to keep our classes small in terms of responsibilities (not necessarily LoC). When a class does too much, it becomes cluttered and has many reasons to change, and too much change usually leads to errors.

I'll use the game of Tic-tac-toe as an example. We could have a partial implementation that looks like this:

class TicTacToe

  def initialize
    @board = [[nil, nil, nil],
              [nil, nil, nil],
              [nil, nil, nil]]
  end

  def winner
    vertical_winner || horizontal_winner || diagonal_winner
  end

  private

  def vertical_winner
    board.transpose.each do |column|
      next if column.include?(nil)
      return column.first if all_cells_are_equal?(column)
    end
  end

  def horizontal_winner
    board.each do |row|
      next if row.include?(nil)
      return row.first if equal_cells?(row)
    end
  end

  def diagonal_winner
    [[board[0][0], board[1][1] + board[2][2]],
     [board[0][2], board[1][1] + board[2][0]].each do |diagonal|
      next if diagonal.include?(nil)
      return diagonal.first if equal_cells?(diagonal)
    end
  end

end

This is clearly an incomplete implementation, but even in this short example we can see how we're hiding the winning logic and juggling several responsibilities: initialization and access to the game board, winning combinations identification and returning the winner. Everything is tied together in the TicTacToe class, it knows about the internal representation of a board (a matrix), and so do the *_winner methods.

A good improvement here would be to extract a Board class that deals with the internal representation of a game board, and knows how to perform certain useful operations on it, like extracting rows, columns and diagonals. Then we can use those in our TicTacToe class to describe our winning logic.

class TicTacTow

  def initialize
    @board = Board.new
  end

  def winner
    vertical_winner || horizontal_winner || diagonal_winner
  end

  def vertical_winner
    board.columns.each do |cells|
      next if cells.include?(Board::EMPTY_CELL)
      return cells.first if all_cells_are_equal?(cells)
    end
  end

  def horizontal_winner
    board.rows.each do |cells|
      next if cells.include?(Board::EMPTY_CELL)
      return cells.first if all_cells_are_equal?(cells)
    end
  end

  def diagonal_winner
    board.diagonals.each do |cells|
      next if cells.include?(Board::EMPTY_CELL)
      return cells.first if all_cells_are_equal?(cells)
    end
  end

  private

  def all_cells_are_equal?(cells)
    cells.uniq.size == 1
  end

end

By extracting the Board and naming it's operations, our code is easier to follow. Now as long as our board object returns a list of plays when calling columns, rows and diagonals, the TicTacToe class doesn't have to know about it's internal representation, and it won't change if/when the Board changes.

This improvement also makes obvious that we have some repetition going on. For every column, row or diagonal, we want to check that the cells contents are equal, and that there are no empty cells. We can perform that check in a single method:

class TicTacToe

  def initialize
    @board = Board.new
  end

  def winner
    winning_combinations.each do |cells|
      next if cells.include?(Board::EMPTY_CELL)
      return cells.first if all_cells_are_equal?(cells)
    end
  end

  private

  def winning_combinations
    rows + columns + diagonals
  end

  def all_cells_are_equal?(cells)
    cells.uniq.size == 1
  end

end

Our Board class could look something like this:

class Board

  def initialize(size)
    @cells = [[nil, nil, nil],
              [nil, nil, nil],
              [nil, nil, nil]]
  end

  def rows
    cells
  end

  def columns
    cells.transpose
  end

  def diagonals
    grid_size = cells.size
    grid_size.times do |i|
      j = grid_size - i - 1
      [rows[i][i], rows[i][j]]
    end.transpose
  end
end

With this separation of responsibilities and a few tweaks we can effectively reuse the elements of the system.

class Matrix

  def initialize(size)
    @cells = size.times.collect { Array.new(size) }
  end

  def rows
    cells
  end

  def columns
    cells.transpose
  end

  def diagonals
    cells.size.times do |i|
      [rows[i][i], rows[i][cells.size - i - 1]]
    end.transpose
  end
end

Now we have written code that can get the rows, columns and diagonals of any n-squared matrix. Compare the ease of reusing this last module vs. what we'd have to go through if we tried extracting that same logic from our first example.

Coupling everything together will usually have a negative impact. Small classes are easier to maintain and extract. It takes a little more time to come up with a proper design but it will save much more time in the long run.