Cory Foy recently wrote an article entitled, Procedural Polymorphism: Is your code really telling you to use an if statement?. Corey Haines has been talking about this same topic for a while. Sandi Metz covers the topic in Practical Object-Oriented Design in Ruby. Ben Rady and Rod Coffin discussed using the Null-Object pattern instead of an if
statement in Continuous Testing with Ruby, Rails, and JavaScript.
That is, very smart people whom I admire agree that, in general,
class DeadCell
def to_s
' '
end
end
class AliveCell
def to_s
'*'
end
end
def print_board
cells.each do |cell|
print cell
end
end
is preferable to
def print_board
cells.each do |cell|
print cell.alive? ? '*' : ' '
end
end
Each time I've read this, I've nodded along and thought, "well, sure." And yet every time I've tried to apply it in practice (even just in my head), I've come up against a stumbling block: when two things interact, something (one of the two or a third mediator) must know about that interaction. In the Conway's-Game-of-Life example above, choosing the first version (object polymorphism) says, "Cells should know about their representation in text," whereas the second says, "The game printer should know about cell state."
I have a hard time understanding why one of those statements is preferable to the other. That is, when there's interaction, one side has to give a little more in the Single-Responsibility Principle fight.
Those who talk about "good OO design" usually have responsiveness as one of their key goals. That is, a change in requirements causes changes in the code commensurate with the scope of change. Minor changes shouldn't require major overhauls and definitely shouldn't break major components. Let's test that.
For our first new requirement, let's say that when printing the Life board, we want live cells to show up in green. We haven't broken the assumption that we're working with ASCII+POSIX here, so we'll use ANSI color codes. This change is trivial:
class LiveCell
def to_s
"\033[32m*\033[39m"
end
end
Yay! Green cells. I wouldn't really call this a win for the OO-polymorphism approach, since the if
version also incurs only a one-line change. But we're certainly no worse off.
Along comes a new requirement: sometimes, we'll be running in an environment that doesn't understand ANSI. In those cases, we should just degrade to plain text. Using if
statements, it might look like this:
class LiveCell
def to_s
char = '*'
Life.use_ansi_colors? ? "\033[32m#{char}\033[39m" : char
end
end
Applying the same principle from earlier, we extract a CellPrinter
and use polymorphism:
class PlainCellPrinter
def print(character, color = nil)
character
end
end
class ANSIColorCellPrinter
COLORS = { green: 32 }
COLORS.default = 39
def print(character, color = nil)
ansi_code = COLORS[color]
"\033[#{ansi_code}m#{character}\033[39m"
end
end
class LiveCell
def print_to(printer)
printer.print '*', :green
end
end
class DeadCell
def print_to(printer)
printer.print ' '
end
end
When the product managers later ask to print games in HTML, adding an
HTMLCellPrinter
is relatively simple. Replacing procedural polymorphism
(if
or case
statements) with OO polymorphism has proven itself!
This solution, however, raises my second qualm with the if
-less approach:
why is a lookup map (COLORS
) better than if
statements? When Ruby searches
through that map, it has to use if
statements internally to find the right
value. Haven't I just moved the if
(admittedly to somewhere outside my
codebase)?
Additionally, while LiveCell
isn't reaching into Life.use_ansi_colors?
, we still need that if
somewhere. In the Environment
approach, it moves to some initialization code:
printer = if current_environment_supports_ansi_colors?
ANSIColorCellPrinter.new
else
PlainCellPrinter.new
end
The product team now comes up with a requirement that doesn't fit very well with the class structure we've developed. They want cells that are overcrowded to print an orange ❉.
Our existing class structure suggests
class OvercrowdedCell
def print_to(printer)
printer.print '❉', :orange
end
end
class ANSIColorCellPrinter
COLORS = {
green: 32,
orange: 33
}
# ...
end
That was easy. OvercrowdedCell
s print perfectly. OO Polymorphism for the win!
Only there's a problem: we don't actually create any overcrowded cells in our
game. The game initializer creates a grid of DeadCell
s and replaces those
indicated by the user with LiveCell
s:
Now I'm stuck. I don't want to move back to an if
statement:
class LiveCell
def print_to(printer)
char, color = overcrowded? ? [ '❉', :orange ] : [ '*', :green ]
printer.print char, color
end
end
But if I want to avoid that if
statement, the initializer needs to somehow
get the right kind of Cell
into each spot. Currently, the initializer
looks like this:
def board_setup
(0..board_view_size).each do |x|
(0..board_view_size).each do |y|
board.push(DeadCell.new(x,y))
end
end
alive_cells.each do |ac|
board.cell_at(ac).make_alive!
end
end
But in the alive_cells.each
iterator, the neighbors of the current cell
haven't necessarily been vifivied, so make_alive!
can't take
overcrowdedness into account. Instead, we have to add a third loop that
adjusts the class of each cell based on its current state:
def board_setup
(0..board_view_size).each do |x|
(0..board_view_size).each do |y|
board.push(DeadCell.new(x,y))
end
end
alive_cells.each do |ac|
board.cell_at(ac).make_alive!
end
(0..board_view_size).each do |x|
(0..board_view_size).each do |y|
board.cell_at([x, y]).readjust_on_initialization!
end
end
end
Now we've introduced readjust_on_initialization!
to the Cell
classes.
Cell
classes already needed to know what type of class they would become
in the next generation, so we're on firm ground on the idea of
"Cell
s know how to map crowdedness to other Cell
classes."
This change, though, introduces the idea of "0th generation"
or "initialization" to a class that didn't formerly care about that.
That is, we've increased the responsibility of all Cell
classes to include
initialization because of a requirement change having to do with printing.
Of course, Sandi Metz suggests an out to these problems:
If you only knew what feature requests would arrive in the future, you could make perfect design decisions today. Unfortunately, you do not. Anything might happen. You can waste a lot of time being torn between equally plausible alternatives before rolling the dice and choosing the right one. Do not feel compelled to make design decisions prematurely. Resist, even if you fear your code would dismay the design gurus... Ask yourself: "What is the future cost of doing nothing today?" - POODR, chapter 2
I see how replacing an if
with OO polymorphism can bring big wins, yet it
(like any other tool or technique) is not a golden hammer / silver bullet
(a "14K hamver"). What I find very difficult is discerning when it's going to
be a net positive and when I should hold off.
One point I didn't mention is that things like
tend to get duplicated across parallel classes. DRYness seems like a sufficient reason to extract that code into a
Printer
object (or elsewhere) at the point that there's duplication. That is, in some cases, I can come to the same result and without abiding by a generalif
-less approach.