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:
class User < ActiveRecord::Base has_one :blog end
class Blog < ActiveRecord::Base belongs_to :user end
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
validates_uniqueness_of :username, to ensure its unique.
Alright thats not that hard. Our first try:
ActionController::Routing::Routes.draw do |map| map.show_blog 'blogs/show/:username', :controller => 'blogs', :action => 'show' end
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
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.
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.
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.
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
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’.