I had an interesting experience at a code retreat with the creator, Corey Haines. I created some code that I felt was really perfect. I didn’t think there was room for much improvement, but it only took Corey a few seconds in passing to find a flaw. It starts with this list of rules for simple design:
- Passes tests – the code should be test-driven, and the tests should all pass.
- No duplication – often known as DRY – don’t repeat yourself. Every distinct piece of information in the system should have one (and only one) representation in the code.
- Expresses intent – the code should be self-explanatory.
- Small – methods, classes, indeed the entire application shouldn’t be any bigger than absolutely necessary.
My Original Version
I won’t explain what this code is supposed to do. That might defeat the point. See if you can figure out which principal I violated with this code. I’ll say it’s not Rule #1, but showing the tests would take up too much room.
def new_status current_status, neighbor_count return :alive if neighbor_count == 3 return current_status if neighbor_count == 2 :dead end
The Problem
Corey asked me one question: what if one of the requirements changes? And there it was. In an attempt to do the most in the fewest lines, I’d over-refactored the method. Not only had I made the method brittle if business requirements should change in the future, I’d factored out the intent of the method itself.
As usual, one good software practice begets another. Test-driven development results in smaller, simpler methods for instance. And in this case, showing intent in your code reduces brittleness. So how do you accomplish this?
The Solution
Express the problem domain in the code itself. Here’s my example, reworked:
def new_status current_status, neighbor_count
return :dead if overpopulated?(neighbor_count)
|| underpopulated?(neighbor_count)
return :alive if population_perfect?(neighbor_count)
current_status
end
def overpopulated? neighbor_count
neighbor_count > 3
end
def underpopulated? neighbor_count
neighbor_count < 2
end
def population_perfect? neighbor_count
neighbor_count == 3
end
This code is longer, but it shows intent much more clearly. You don’t even need to know what “overpopulated” is to understand what the method is doing. But if you want to know, or need to change it, it’s easy. In fact, we’re passing neighbor_count around a lot, so it looks like it’s time to abstract this into a class:
class Cell
def initialize current_status, neighbor_count
@current_status = current_status
@neighbor_count = neighbor_count
end
def next_status
return :dead if overpopulated? || underpopulated?
return :alive if population_perfect?
@current_status
end
private
def overpopulated?
@neighbor_count > 3
end
def underpopulated?
@neighbor_count < 2
end
def population_perfect?
@neighbor_count == 3
end
end
Now the code is much more readable, and understandable. We’re now clearly showing intent. And now, just for fun, the tests:
class CellTest << Test::Unit::TestCase
def test_should_die_when_alive_and_overpopulated
cell = Cell.new :alive, 4
assert_equal :dead, cell.next_status
end
def test_should_die_when_alive_and_underpopulated
cell = Cell.new :alive, 1
assert_equal :dead, cell.next_status
end
def test_should_live_when_alive_and_perfect_population
cell = Cell.new :alive, 3
assert_equal :alive, cell.next_status
end
def test_should_stay_alive_by_default_when_alive
cell = Cell.new :alive, 2
assert_equal :alive, cell.next_status
end
def test_should_die_when_dead_and_overpopulated
cell = Cell.new :dead, 4
assert_equal :dead, cell.next_status
end
def test_should_die_when_dead_and_underpopulated
cell = Cell.new :dead, 1
assert_equal :dead, cell.next_status
end
def test_should_live_when_dead_and_perfect_population
cell = Cell.new :dead, 3
assert_equal :alive, cell.next_status
end
def test_should_stay_dead_by_default
cell = Cell.new :dead, 2
assert_equal :dead, cell.next_status
end
end
