Skip to content

Instantly share code, notes, and snippets.

@jamesarosen
Last active July 19, 2020 19:53
Show Gist options
  • Star 3 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save jamesarosen/5881146 to your computer and use it in GitHub Desktop.
Save jamesarosen/5881146 to your computer and use it in GitHub Desktop.
On Procedural Polymorphism: I like the idea of replacing ifs with Polymorphism, but I'm not quite comfortable with it yet.

Background

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.

Printing in Conway's Game of Life

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."

Qualm One

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.

Responsiveness

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.

Adding Color

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.

Multiple Environments

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!

Qualm Two

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

New Cell States

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. OvercrowdedCells 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 DeadCells and replaces those indicated by the user with LiveCells:

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

Qualm Three

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 "Cells 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.

Pragmatism

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

Summary

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.

@jamesarosen
Copy link
Author

One point I didn't mention is that things like

Life.use_ansi_colors? ? "\033[32m#{char}\033[39m" : char

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 general if-less approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment