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’.