Err the Blog Atom Feed Icon
Err the Blog
Rubyisms and Railities
  • “ActiveRecord Variance”
    – PJ on July 31, 2006

    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?

  • evan, about 14 hours later:

    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.

  • PJ, about 15 hours later:

    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?

  • evan, about 15 hours later:

    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.

  • Lewis, about 24 hours later:

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

  • PJ, 2 days later:

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

  • Tim, 2 days later:

    .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.

  • josh, 3 days later:

    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.

  • Chris, about 1 month later:

    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.

  • Eight people have commented.
    Chime in.
    Sorry, no more comments :(
This is Err, the weblog of PJ Hyett and Chris Wanstrath.
All original content copyright ©2006-2008 the aforementioned.