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 all_cells_are_equal?(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 all_cells_are_equal?(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.
An improvement could 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 even more time in the long run (plus, it’s fun).