bustin' out JOINs

Jared Carroll

Here at t-bot we scrutinize every line of code. Here’s one from the Rails’ world.

Ok, pretend we have a site where each registered user has their own blog. Naturally we’d have the ability for you to view someone’s blog. Following a design where each model has its own controller that handles all its CRUD, we put this feature in the #show action like so:

user.rb

class User < ActiveRecord::Base
  has_one :blog
end

blog.rb

class Blog < ActiveRecord::Base
  belongs_to :user
end

blogs_controller.rb

class BlogsController < ApplicationController
  def show
    @blog = Blog.find params[:id]
  end
end

The URL of that action would look something like ‘/blogs/show/1’.

But wait. That URL is just not pretty enough. The client asks it to be changed to ‘/blogs/show/chad’, ‘chad’ being the username of the user that the blog belongs to. When users register we also validates_uniqueness_of :username, to ensure its unique.

Alright thats not that hard. Our first try:

routes.rb

ActionController::Routing::Routes.draw do |map|
  map.show_blog 'blogs/show/:username', :controller => 'blogs', :action => 'show'
end

blogs_controller.rb

class BlogsController < ApplicationController
  def show
    @user = User.find :first, :conditions => ['username = ?', params[:username]]
    @blog = @user.blog
  end
end

Ugly. I don’t like setting instance variables to objects that I can navigate to from other instance variables I’ve already set. I already found the User object with the given username, I can then ask it for its blog. I like keeping the actions short and the instance variables to the absolute minimum that I need. How about making user a local variable? Nah, I don’t like local variables in actions.

Let’s try again.

blogs_controller.rb

class BlogsController < ApplicationController
  def show
    @user = User.find :first, :conditions => ['username = ?', params[:username]]
  end
end

Nope, still don’t like it. Like I said earlier, I like to let each model have its own controller that handles all its CRUD e.g. Blog model, BlogsController. I expect the model’s controller to be only working with objects of that type e.g. the BlogsController should only be dealing with Blog objects but now the #show action find’s a User object. That doesn’t seem right to me.

Here we go again.

blogs_controller.rb

class BlogsController < ApplicationController
  def show
    @blog = Blog.find :first,
        :joins => 'inner join users on users.id = blogs.user_id',
        :conditions => ['users.username = ?', params[:username]]
  end
end

Oooo. No more User object, I like that. The BlogsController is now only working with Blog objects, another plus. But…the ‘joins’ keyword parameter.

From the Rails doc

:joins: An SQL fragment for additional joins like LEFT JOIN comments ON comments.post_id = id. (Rarely needed).

Rarely needed….hmmm. Some of you might think I’m doing this for performance. After all, if I loaded just the User object that would be 1 query, then when I navigated to the user’s blog that would be another query. Two queries compared to one. I also could get the same performance as the query using ‘joins’ by using the ‘include’ keyword parameter in my #find to eagerly fetch the user object’s associated blog, but nah I don’t feel like doing that because I’d still be loading a User object in the BlogsController and that feels wrong. I’m not doing this for performance reasons, its purely for clarity.

The bottom line is: don’t trade pretty URLs for ugly code even if that means you have to bust out ‘joins’.

About thoughtbot

We've been helping engineering teams deliver exceptional products for over 20 years. Our designers, developers, and product managers work closely with teams to solve your toughest software challenges through collaborative design and development. Learn more about us.