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
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?
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