Posts Tagged ‘show intent’

Show Intent with Better Naming

March 8, 2011

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:

  1. Passes tests – the code should be test-driven, and the tests should all pass.
  2. 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.
  3. Expresses intent – the code should be self-explanatory.
  4. 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

Follow

Get every new post delivered to your Inbox.