pragmatist
Patrick Joyce

January 29, 2014

Executable Comments: `say_with_time`

I have a rather well-documented distaste for comments.

There are times where you want explanatory information in code. One of my favorite ways to add that information while lessening the risk of it becoming stale is to replace a comment with executable code.

I was reminded of this recently (ed: I actually sketched this draft out 7 months ago) while working on a Rails migration. The code creates a join table and then adds some default data. My first pass looked something like:

def self.up
  create_table :post_category_mappings do |t|
    t.integer :post_id
    t.integer :category_id
  end

  # Populating initial post => category relationships
  post_category_mappings = YAML.load_file("#{RAILS_ROOT}/db/initial-post_category-mapping.yml")
  post_category_mappings.each do |category_id, post_ids|
    Category.find(category_id).post_ids << post_ids
  end
end

That isn't the best code I've ever written, but its reasonable. The comment functions as a section heading. It tells you that we're done with the normal work of a migration (schema changes), something different is happening now, and you should pay attention.

Migrations also don't typically change after they're checked in, so its really unlikely that the comment will fall out of sync with the code.

I don't hate that comment.

However, Rails provides a nice mechanism for converting that comment into code—and getting some bonus functionality: say_with_time

def self.up
  create_table :post_category_mappings do |t|
    t.integer :post_id
    t.integer :category_id
  end

  say_with_time "Populating initial post => category relationships" do
    post_category_mappings = YAML.load_file("#{RAILS_ROOT}/db/initial-post_category-mapping.yml")
    post_category_mappings.each do |category_id, post_ids|
      Category.find(category_id).post_ids << post_ids
    end
  end
end

We preserve the section-header benefit of calling attention to a logical block of code; plus we now also get pretty output and timing information when the migration runs.

This is an improvement.

This is just one minor way in which you can replace comments with code, but every time I come across say_with_time it makes me smile.

Update (2013-01-31)

I should note that mixing data population into your migrations is generally A Bad Idea™.

If the population portion of the migration fails—and it can happen due to differences in dev / QA vs. production—your DB will be left in an inconsistent state. You won't be able to run rake db:rollback as the migration hadn't completed so the "current" version will be the previous version. You also won't be able to run the migration again because the table will have already been created so running the up migration will fail.

You'll be forced to mess around with the DB directly to restore things. We use migrations to avoid this.

You could make a fairly compelling argument for wrapping all migrations in transactions... but that introduces a whole host of other complexities.

say_with_time is still awesome, but you should probably put the data population code in its own migration—or leave it out of migrations entirely.

More Articles on Software & Product Development

Agile With a Lowercase “a”
”Agile“ is an adjective. It is not a noun. It isn’t something you do, it is something you are.
How Do You End Up With A Great Product A Year From Now?
Nail the next two weeks. 26 times in a row.
Build it Twice
Resist the urge to abstract until you've learned what is general to a class of problems and what is specific to each problem.