The Quality of Outsourcing October 10th, 2007

We can thank Eric Mill for forwarding this code. Models have been changed to protect the innocent. 

<% if Car.find(@driver.car_id).car != nil %>

Model, VIEW, Controller

You NEVER put finders in your view. I know exceptions, but explaining them will be enough justification for some idiot. Look, you put a finder in your view, you're dead to me.

Did they even watch the screencast?

It's remarkable they don't know ActiveRecord relationship macros. That feature converts people to Rails. The column even follows naming conventions! All it takes:

class Driver
  has_one :car
end

Then you've got:

<% if @driver.car.car != nil %>

Which only leaves the column name. Wouldn't 'name' be more descriptive?

Truth isn't hard

Last, the coder doesn't know the Ruby definition of truth. Anything except false or nil is true. That leaves us with:

<% if @car.name %>

Which doesn't cover an empty string. You really want:

<% unless @car.name.blank? %>

The moral: don't outsource your code overseas.

Mysterious Constants September 24th, 2007

<%= listing.sales_tier_summary if listing.tier == '999' %>

If you're like me, you get lost at the magical '999'. Maybe it means 'summarizable'. If so, it should be:

<%= listing.sales_tier_summary if listing.summarizable? %>

That way it's self documenting, and when the constant changes you aren't hunting through your views to update it. Welcome to MVC.