malvasia bianca

I finished adding multiuser support to my memory project this weekend. I’d added the ability for users to log in and out while I was on vacation this summer, but I hadn’t actually linked up the users with their lists of items to memorize.

The relevant models are a User model and an Item model. A user has_many :items; part of the Item class definition was the following:

class Item < ActiveRecord::Base
  ...
  def self.next
    find(:first,
         :order => "next_review_time",
         :conditions => pending_item_condition)
  end

  def self.pending_review_count
    count :conditions => pending_item_condition
  end

  def self.pending_item_condition
    ["next_review_time <= ?", Time.now.utc]
  end
  ...
end

As written, those would, for example, add up the total count of all users’ pending items; I wanted instead to be able to ask those questions on a per-user basis.

The has_many declaration gives me a collection user.items for each user. And if I pass a block to has_many, I can add methods to that collection; that seemed more stylish than, for example, adding a user parameter to the above methods. So I modified my User unit tests to make sure that there were items owned by multiple users, and copied over the Item unit tests for the above methods, replacing, e.g. calls to Item.next with calls to user.items.next.

Which I expected to fail, complaining about an undefined method “next”. But not only did the tests not fail for that reason, they in fact passed, ignoring the items for other users! At first, I assumed I’d made a mistake in my tests, but no: when I replaced the calls to user.items.next with calls to Item.next, they failed as expected. So the functionality I wanted really did seem to be working without my having to lift a finger.

But how? I could imagine some sort of method_missing implementation that forwarded functions on the collection to functions on Item. But if it were doing that, how was the functionality getting restricted to items owned by the user in question? I would hope that it wasn’t doing a broader search in SQL and then subsequently ditching inappropriate items in Ruby; sure enough, a look at the logs shows that the SQL queries that are issued do include the user_id restriction.

I haven’t verified it yet, but the only hypothesis that I’ve come up with so far is that the collection objects that are returned by user.items start off life as clones of Item; so they have all of the class methods on Item. And then some key method (find, say) is overridden so that the user restriction always gets applied, and all other lookup methods are implemented in terms of that key method, so they get the restriction in question for free.

Which, once I thought about it, makes sense. Consider Item, not as a class but as an object: you can think of it as a collection object, namely the collection of all items. With that in mind, it makes sense for other objects that are also collections of items to be closely related to it, and if we can express that tight linkage in terms of the implementation, by thinking of Item as the primordial collection of items, so much the better!

An interesting journey. At first, I thought I must be writing bad tests; then I thought I was seeing magic; and eventually I came around to a realization that what I was seeing made a lot of sense conceptually, so I was happy that I was working with a language and a framework where I could get such sensible behavior for free. And, no matter what, it was a useful reminder that there’s a reason why the TDD cycle is red, green, refactor, not just green (with simultaneous test+implementation changes), refactor: if I’d done the latter, I would have missed a useful learning opportunity.

Though there’s a postscript to the story. I’d learned that my original implementation idea was unnecessary; the question remains, though, whether the original implementation idea was a good one or not. And, after giving it some thought, I decided that I preferred having these methods in the User model rather than in the Item model. If I left them in Item, it would be easy to write code representing concepts like “what’s the next item that any user will be asked?” or “what’s the total pending item count across all users?” Which aren’t natural questions to ask, so I was setting myself up for bugs by leaving those methods in Item.

So, with my tests in place, I moved them to User:

class User < ActiveRecord::Base
  ...
  has_many :items do
    def next
      find(:first,
           :order => "next_review_time",
           :conditions => pending_item_condition)
    end

    def pending_review_count
      count :conditions => pending_item_condition
    end

    def pending_item_condition
      ["next_review_time <= ?", Time.now.utc]
    end
  end
  ...
end

Leaving me where I expected to end up when I began, but having taken a much more interesting route.