Rails 4.0 to 4.1 Upgrade

When updating from Rails 4.0 to 4.1, I ran into two issues:

The first was that has_and_belongs_to_many associations now seem to require a primary key on the join table. Easy enough to solve – just add a primary key to your join table.

# As a migration:

class AddPrimaryKeyToMissionsVendors < ActiveRecord::Migration
  def change
    add_column :missions_vendors, :id, :primary_key
  end
end

The second was that ActiveRecord #count queries have changed behaviour a bit. Before, the select clause defaulted to SELECT(*). So, even if you had something like:

User.select(:id, :username, :favourite_color).count

The query would end up in SQL as:

SELECT count(*) from users;

The newer version generates the (more accurate) SQL:

SELECT count(users.id, users.username, users.favourite_color) from users;

In my case this actually broke some places where I was implicitly relying on count(*). To fix this, I just changed count to count(:all).

Decoupling from Rails

Here is a great talk called Decoupling from Rails by the late Jim Weirich.

This talk is most famous for being taken up by David Heinemeier Hansson as an example of “test induced design damage” during his Is TDD Dead? conversation series with Kent Beck and Martin Fowler. Putting that debate aside, though, here are some things this talk got me to understand more deeply or start thinking about:

Differentiating between “framework” code that we don’t own, and “application” code that we do. It’s interesting to think about defining clear boundaries between these two types of code, and about decoupling them as much as possible. (It’s worth thinking about even if you don’t end up doing it in the end!).

Many people encourage writing wrappers around external APIs we use in our application. It’s considered good practice to write and interact with a PaymentGateway wrapper we do own, which wraps a Braintree gem which we don’t own, rather than interacting directly with Braintree throughout the application. This is the same concept, except applied to Rails.

Injecting Rails objects as a dependency into plain old ruby objects. Rails is still the “context” in which other things are done, but you can dynamically swap in other contexts that respond to the same methods the Rails context does:

class EmployeesController < ApplicationController
  def create
    CreateRunner.new(self)
  end
end

class CreateRunner
  attr_reader :context

  def initialize(context)
    @context = context
  end

  def do_something
    context.do_something
  end
end

Wrapping ActiveRecord models up in a business logic layer using simple delegator. This is something I’ve heard a lot about but haven’t personally started doing yet. It was great to see a live-coding example of making it work. (At around minute 56 of the talk).

Some nice text-editing moments. Jim had something automatic set up for extracting variables into methods.

I hope you enjoy the talk as much as I did!

From Good to Great

The third great conference talk I want to share is Ben Orenstein’s Aloha Ruby Conf 2012 talk called Refactoring from Good to Great.

Ben shows some common refactoring patterns by doing live coding on stage. He not only covers what he’s doing, but he also talks a lot about the why. I learned a tremendous amount from this talk.

Here are some of the topics he covers:

  • Extracting temporary variables to query
  • Tell don’t ask
  • Reducing parameter coupling by extracting data clumps into an object
  • Feature envy

And a few more.

My favourite quotes from the talk are:

I’m starting to think, these days, that methods longer than a line are a code smell.

Most intermediate object oriented programmers are too reluctant to extract [small] classes.

That’s what’s hard about software, is responding to change. I saw a keynote by Dave Thomas, and he said “The only thing worth worrying about when you look at code is: is it easy to change?”

A therapy session with Katrina Owen

This week I’m posting a mini-series on great conference talks.

Today’s installment is a talk by Katrina Owen called Therapeutic Refactoring.

I’ve watched and re-watched, and re-re-watched this talk several times over the past year. Each time, it just seems to get better and better. I understand more of it, and notice and learn new things.

It’s also got great slides, a lot of which are from Hyperbole and a Half.

Katrina is also running exercism.io, a site where you can complete small coding problems and get peer feedback on them. I’ve been enjoying participating in that recently as well.

Enjoy the talk!

Sandi Metz does Magic

This week, I’ll be posting links to some great conference talks that I’ve either attended in person or seen online.

Sandi Metz gave a really great talk on proper testing, called The Magic Tricks of Testing at RailsConf 2013. I can’t wait to put some of her advice into action!

Here are some memorable quotes from the talk:

Integration tests are proof that the beast is alive!

And:

This is our job… To find the simplicity that lives at the heart of complexity.

Best Practices for Loading Code

I’ve made it a whole year without thinking about best practices for loading code. I’ll blame that on the wonderful magic of Rails, for taking care of details like this for me behind the scenes.

I was working on a very small Ruby program and found myself wondering if this is a code smell:

# in spec/mars_rover_spec.rb

require 'spec_helper'
require_relative '../lib/mars_rover'
require_relative '../lib/rover_coordinates'
require_relative '../lib/rover_position_tracker'

describe MarsRover do
  ...
end

To make a long story short, the answer is YES. In this example, our spec files not only know about their dependencies, they even know where those dependencies physically reside on disk in relation to themselves.

Let’s refactor.

I bet most if not all our spec files are going to be requiring ‘spec_helper’. Let’s delete that line and put that information into configuration:

  # in .rspec
  --require spec_helper

All of our source code lives in lib/. Let’s abstract that knowledge into a small file that we can require on application startup:

  # in environment.rb
  $LOAD_PATH.unshift(File.expand_path("../lib", __FILE__))
  # in spec/spec_helper
  require_relative '../environment'
  # in Rakefile
  require_relative './environment'

Cool. Now our spec looks like this:

# in spec/mars_rover_spec.rb

require_relative 'mars_rover'
require_relative 'rover_coordinates'
require_relative 'rover_position_tracker'

describe MarsRover do
  ...
end

mars_rover, rover_coordinates, and rover_position_tracker are now required logically, just like an external gem, instead of relatively or absolutely.

The last thing we need to consider is why our spec relies on rover_coordinates and rover_position_tracker. Why doesn’t it just rely on mars_rover, which after all is the only class it is testing?

Even the MarsRover class itself doesn’t need to require rover_coordinates and rover_position_tracker, since those dependencies are injected on initialization:

class MarsRover
  def initialize(rover_coordinates, rover_position_tracker)
    @rover_coordinates = rover_coordinates
    @rover_position_tracker = rover_position_tracker
  end
end

The answer is that while those dependencies are injected into the MarsRover class, in the spec, we need to instantiate them as setup, like this:

# in spec/mars_rover_spec.rb

require_relative 'mars_rover'
require_relative 'rover_coordinates'
require_relative 'rover_position_tracker'

it "Has coordinates and a tracker" do
  coordinates = RoverCoordinates.new()
  tracker = RoverPositionTracker.new()
  rover = MarsRover.new(coordinates, tracker)
  # ... test stuff about the rover
end

I tend to call it a day once we’ve gotten this far. I’ve never done it myself, but I believe we could avoid requiring all this extra code by using rspec doubles:

# in spec/mars_rover_spec.rb

require_relative 'mars_rover'

it "Has coordinates and a tracker" do
  coordinates = instance_double("RoverCoordinates", to_s: "1, 4")
  tracker = instance_double("RoverTracker", rover_positions: [[1, 3], [4, 99]])
  rover = MarsRover.new(coordinates, tracker)
  # ... test stuff about the rover
end

Source: Ruby Tapas Episode # 242, by Avdi Grimm. If you don’t know about Ruby Tapas yet, you should check it out! Any errors in this blog post are probably mine, not Avdi’s.

Sharing HTML and PDF code with the facade pattern

In the system I wrote for my former company, several pages needed to be available in both HTML and PDF formats.  As usual, there was a tradeoff to consider:

If we exported the HTML document directly to a PDF using wicked_pdf, then there was less code to write, but the resulting PDF wouldn’t look as good.

If we wrote the PDF code separately from the HTML using Prawn, then the PDFs looked nicer, but then we had twice as much code to maintain.

We went the latter route, and ended up with a lot of code that felt redundant:

  # in HTML view
  - if @order.items.active.any?
    h3 Active items
    table
        tr
          th Item SKU
          th Item quantity
      - @order.items.active.each do |item|
        tr
          td = item.sku
          td = item.quantity


  # in Prawn PDF
  if @order.items.active.any?
    text Active items
    table [["Item SKU", "Item quantity"] + @order.items.active.map do |item|
      [
        item.sku,
        item.quantity
      ]
    end
  end

Some of this duplication can be mitigated, though.  We’ll do this using the Facade Pattern:

class ReportFacade
  def initialize(order)
    @order = order
  end

  def item_section_header
    "Active Items"
  end

  def show_items?
    items.any?
  end

  def items
    @order.items.active
  end
end

This centralizes the logic for:

1. If the section should be shown at all: @order.items.active.any?
2. What items should be shown: @order.items.active
3. What the section header text should be: "Active Items"

Now, in our controller, we switch

  def show
    @order = Order.find(params[:id])
  end

To:

  def show
    order = Order.find(params[:id])
    @report_facade = ReportFacade.new(order)
  end

And access @report_facade in our views.

  # in HTML view
  - if @report_facade.show_items?
    h3 = item_section_header
    table
        tr
          th Item SKU
          th Item quantity
      - @report_facade.items.each do |item|
        tr
          td = item.sku
          td = item.quantity

  # in Prawn PDF

  if @report_facade.show_items?
    text item_section_header
    table [["Item SKU", "Item quantity"] + @report_facade.items.map do |item|
      [
        item.sku,
        item.quantity
      ]
    end
  end

The advantage here is that it is harder for these two formats to get out of sync with one another. Changing the header in the ReportFacade object will change it in both the HTML and the PDF versions.

The disadvantage is that it makes for even more code overall, and when trying to understand the HTML or PDF code, you have to constantly reference the ReportFacade class.

It’s not so clear to me whether this is an overall win or not. What do you think? Is this worth doing?