Stinky Magic
This post was written in response to http://modernsage.wordpress.com/2011/02/05/rails-3-0-arel-acrobatics/. The original post has code examples that are clearly for illustrative purposes and not from the codebase (code where AR models are not descending from AR::Base, for example), so it’s possible that some important details have been left out. However, for the sake of conversation (and because sometimes it’s just fun to write a nerdy blog post), I wanted to respond.
In that blog post, my buddy Chris is doing some stuff with Arel/AR that, in my opinion, is backwards. It’s causing some confusion that led him to extend ActiveRecord::Base in a way that I feel is unnecessary, and potentially dangerous down the road.
Chris wants to do stuff like:
Forum.mine.posts.comments
The intent is that he wants to access all the comments owned by a post owned by a forum owned by a particular user. Working off the Forum model, he wants to scope things through the ‘mine’ scope, which scopes the Forums to an authenticated user.
Two problems
First, Forum.mine is a backwards way of accessing records belonging to an authenticated user. Normally, a Rails app would authenticate a user and provide helpers at the view and controller level to access that user. You’ll see this in a lot of apps as the method ‘current_user’.
The problem with Forums.mine is that in order for it to work, there has to be code inside the model that goes back out to the authentication system. Authentication is the controller’s domain. It’s an awkward coupling of concerns that causes your code to get all tangled up.
A much more straightforward way to retrieve objects is to have the user be at the root when going left to right. The common way to get Forums owned by a person at the controller level would be:
current_user.forums
The second problem is that:
Forum.mine.posts.comments
just doesn’t make sense.
Posts, as a collection, don’t have comments. A single Post has comments. The concept of Comments that belong to many Posts doesn’t exist here. It’s a confusion I’ve seen in new and experienced ActiveRecord developers alike.
This is much more straightforward:
current_user.forum_comments
In a github repo, I’ve put an example of how I would write this. User#forum_comments, in this case, uses a has_many :through relationship to get at the posts, and then does a map and a second query to get the comments.
class User < ActiveRecord::Base
has_many :forums
has_many :posts, :through => :forums
def forum_comments
Comment.where(:post_id => posts.map(&:id))
end
end
Because Chris didn’t want to do this, he implemented an extension of ActiveRecord::Base that allows it to work:
Forum.mine.posts.comments
It goes through and creates all the necessary scoping to allow that query to return comments. Even though I don’t have any examples, I’m pretty this will break down as more complicated relationships are introduced.
Stinky Magic
The last paragraph of Chris’ post says:
You can use it with a single association or with a monster hierarchy of the form A -> B -> C -> D -> E -> F ->DRY, recursive, scalable. Now that’s what I call magic.
In my opinion, this is horribly dangerous. If you start doing this stuff, and A, B, C, D, and E grow to be a couple million rows (which is not very uncommon at all nowadays) you’re gonna have a very non-scalable join to deal with.
I think a developer should err on the side of verbosity and explicitness when defining relationships. If you really need to do A -> B -> C -> D -> E -> F a lot, that’s a sign that some de-normalization may be justified. Assuming that -> denotes has_many, F would have an e_id on it. In this case, adding an a_id would allow you to go A -> F directly and much faster.
A little bit of CS (read: Kool-Aid)
Another reason to go the “user.forum_comments” route instead of the “Forum.mine.photos.comments” route is that it invites developers to constantly violate the Law of Demeter. I didn’t go to school for CS, but I’ve come to respect this rule over my years of Ruby development. It’ll help to keep your code readable, and easier to mock behavior for testing.
