This post originated from an RSS feed registered with Ruby Buzz
by Dave Hoover.
Original Post: Relentless Simplicity with ActiveRecord
Feed Title: Red Squirrel Reflections
Feed URL: http://redsquirrel.com/cgi-bin/dave/index.rss
Feed Description: Dave Hoover explores the psychology of software development
The Active Record pattern is a wonderfully simple and sufficient approach to ORM on many projects. I first used it in Java and it worked well on a small project. Yet without mixins, using Active Record is very limiting and/or awkward. It wasn't until I starting using ActiveRecord in Rails that I saw just how powerful this pattern could be when applied within a more flexible, readable language. Where nasty getter/setter hell and XML configurations are the standard in Java, we get clean, readable classes and objects in Ruby.
One of the things we do at Obtiva is provide coaching and development for Rails projects that are in trouble. I call them rescue missions. We've found that the downside to Rails' power and simplicity is that people can crank out functionality with minimal understanding of how to write maintainable, scalable software. It's only a matter of time until they hit a wall, and that's where we step in. We find people usually get into trouble with ActiveRecord when they allow themselves to write ugly code. I'm sure that there are some cases where ActiveRecord doesn't do what you need and an ugly solution is required, but I think those cases are exceptionally rare for most Rails apps.
During one of our rescue missions, I discovered a fat controller with lots of calls to find_by_sql. Again, I'm sure there are cases where this nasty method is needed, perhaps for optimization, or for some odd cases that I can't imagine (since I've never used it), but this was just plain ActiveRecord abuse. Most of us aren't aren't writing abusive ActiveRecord code, but many of us need to be more relentless in our pursuit of clean, readable, ActiveRecord usage.
We're kicking off a Rails e-commerce project so I downloaded a copy of the Rails e-commerce book to brush up on ActiveMerchant. By the way, I recommend the book to anyone coming to Rails with an immediate need to write an e-commerce site. It's also a good book for anyone wanting to see an example of TDD in Rails. But on page 175, I found a snippet of code that spurred me to write this post ... it wasn't ugly ActiveRecord code, but I would go so far as to say that it is just plain bad.
class Post < ActiveRecord::Base
belongs_to :category
def self.find_all_in_category(category)
category = Category.find_by_name(category)
self.find :all, :conditions => "category_id = #{category.id}"
end
...
end
I can read this code and understand what the author was trying to do. The method is returning all of the Posts for a given Category name. But what happens if the call to Category.find_by_name returns nil? And why would a simple thing like this require 2 separate calls to the database?
SELECT * FROM categories WHERE (categories.`name` = 'foo') LIMIT 1
SELECT * FROM posts WHERE (category_id = 1)
This is an example of not pushing hard enough to keep your code clean and simple. The first solution you can think of isn't necessarily the simplest. Don't be satisfied with anything less than elegance when you're using ActiveRecord and you'll be much better off. Here's how I would rewrite that little method:
class Post < ActiveRecord::Base
belongs_to :category
def self.find_all_in_category(category_name)
category = Category.find_by_name(category_name, :include => :posts)
category ? category.posts : []
end
...
end
Which, thanks to the eager loading of :posts, would result in just 1 call to the database, and (to me) reads better.
Previous Topic
