GIANT ROBOTS SMASHING INTO OTHER GIANT ROBOTS

Written by thoughtbot

bustin' out JOINs

Here at t-bot we scrunitize 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 just 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. 2 queries comparied to 1. 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'.