ActiveRecord Variance — err.the

So, I’ve been developing Rails apps for quite some time, but I never really noticed the following issue:

>> User.find_by_id(10)
=> nil
>> User.find(:first, :conditions => ["id = ?", 10])
=> nil
>> User.find(10)
ActiveRecord::RecordNotFound: Couldn't find User with ID=10

I’d love to know the rationale behind the decision to make find_from_ids throw a RecordNotFound exception when the equivalent methods return nil.

At any rate, lets fix this. Here’s the existing AR code (sans comments):

def find_one(id, options)
  conditions = " AND (#{sanitize_sql(options[:conditions])})" if options[:conditions]
  options.update :conditions => "#{table_name}.#{primary_key} = #{quote(id,columns_hash[primary_key])}#{conditions}"
  if result = find_every(options).first
    result
  else
    raise RecordNotFound, "Couldn't find #{name} with ID=#{id}#{conditions}"
  end
end

And here’s code that makes more sense:

def find_one(id, options)
  conditions = " AND (#{sanitize_sql(options[:conditions])})" if options[:conditions]
  options.update :conditions => "#{table_name}.#{primary_key} = #{quote(id,columns_hash[primary_key])}#{conditions}"
  find_every(options).first
end

Which now gives us:

>> User.find(10)
=> nil

Ah, much better. Note that the find_some method is also afflicted with the same issue, but I’ll leave that as an exercise to the reader to fix.

I can only presume that there’s a reason for this since someone took the time to write the exceptions even though the SQL generated is exactly the same amongst the methods. Core?

I think the idea is that when you .find(), you know the primary key. So you always expect your record to exist. If that is the case, you want an exception if your expectation is not met.

However, when you use find_by_x or :conditions, they may be programmatically generated, and it’s unlikely that you would already be able to guarantee the existence of the record—if you could do that, wouldn’t you probably also know the primary key? So the fact that find_by_id etc. return nil is an artifact of the primary key’s default being (but not necessarily) ‘id’. .find() always searches on the primary key, which should always exist; but find_by_id just searches on the column id whether or not it is the primary key.

That was the only conclusion I could come up with, but I’m still not sure what you’re gaining by throwing the exception.

Whether it be find() or find_by_id, it’s all syntatic sugar anyways, so why is Rails taking it upon itself to return something different depending on the sort of sugar?

Yeah I see your point. I like it the way it is though, but then I also use save! and create! all the time because I like to handle errors with exceptions. Maybe there should be .find! and .find_by_id! methods instead.

Actually, I like that idea a lot, come to think of it.

If you look in the rails book, DHH gives a rationale for the difference. Which basically the same as the reason Evan gave.

Love the .find! idea, it’s much more consistent with the rest of ActiveRecord.

.find! would be consistent with .save!, but neither are very rubyesqe. The exclamation should tell you if the method modifies the target, rather than in this case to indicate if a method returns nil or throws an exception.

Throwing the exception allows you to rescue it in the controller and show a 404 page for a nonexistent record, or whatever you want to do. Check out rescue\_action\_in\_public in ActionController::Base.

There’s a discussion on the rails-core mailing list about this right this very second! Check it out—find_by_horse_name!, etc is being considered.

Sorry, no more comments :(