134

Possible Duplicate:
Model.find(1) gives ActiveRecord error when id 1 does not exist

If there is no user with an id of 1 in the database, trying User.find(1) will raise an exception.

Why is this?

Community
  • 1
  • 1
Kirschstein
  • 14,570
  • 14
  • 61
  • 79

2 Answers2

240

Because that's the way the architects intended find(id) to work, as indicated in the RDoc:

Find by id - This can either be a specific id (1), a list of ids (1, 5, 6), or an array of ids ([5, 6, 10]). If no record can be found for all of the listed ids, then RecordNotFound will be raised.

If you don't want the exception to be raised, use find_by_id, which will return nil if it can't find an object with the specified id. Your example would then be User.find_by_id(1).

runako
  • 6,132
  • 2
  • 25
  • 15
  • But what's the advantage of it returning RecordNotFound? Why would someone want this behaviour instead of just returning nil? – Arcolye Nov 27 '10 at 07:59
  • 3
    RecordNotFound is raised, not returned. This allows the caller's control flow to be different, because you don't have to check the return value being nil (you'd use a begin/rescue block instead). – runako Nov 29 '10 at 23:48
  • 4
    That still doesn't explain why it's the default behaviour where in most all other parts of ActiveRecord, the default behaviour is to return nil or false for failures, and leave the exception control flow to methods ending in a bang (#save!). – Marten Veldthuis Aug 12 '11 at 08:07
  • 46
    I think the reason for raising the error is so the base controller can catch it and show show the 404 page. If you hit /widgets/2 and there is no widget with id==2, then you get a 404, which IMO makes sense. – Sammy Larbi May 01 '12 at 19:07
  • `find_by_id` is deprecated in Rails 4 :( https://github.com/rails/activerecord-deprecated_finders – Tobias Cohen Jan 21 '14 at 22:31
  • @TobiasCohen That's not true. From the link you posted: "Note that `find(primary_key)`, `find_by...`, and `find_by...!` are not deprecated." – Nick Mar 31 '14 at 15:06
  • 8
    @Nick `find_by_id(1)` is deprecated indeed. But you could use `find_by(id: 1)`. – Fatih Jul 01 '15 at 09:23
  • @Faith Yup, you're absolutely right. Must have read it wrong at the time. – Nick Jul 02 '15 at 12:16
  • What if there is an error saying that `find_by_id` doesn't exist? – Gcap Sep 03 '15 at 00:47
  • 7
    The explanation of how to avoid the exception is useful, but nothing here actually answers the question of why it is the way it is. In other words, "because the people who made it wanted it that way" is not a useful answer, and neither is "the documentation says it is that way". – cesoid Sep 21 '18 at 18:32
-8

Further to runako's explanation, it's actually pretty useful to have the choice of whether an exception is raised or not. I'm working on a blog application and I wanted to add support for viewing the next or previous blog entry. I was able to add two instance methods to my Post model that simply return nil when you try to get the previous post when viewing the first post, or the next post when viewing the last post:

def next
  Post.find_by_id(id + 1)
end

def previous
  Post.find_by_id(id - 1)
end

This avoids my helper code which conditionally generates the Previous Post/Next Post links from having to handle the RecordNotFound exception, which would be bad because it would be using an exception for control flow.

John Topley
  • 113,588
  • 46
  • 195
  • 237
  • Wouldn't this be pretty fragile if your records in your DB didn't have sequential id's? – Pete Sep 30 '11 at 08:39
  • @zoltarSpeaks It would, but in practice they do have sequential IDs, so it's a total non-issue. – John Topley Sep 30 '11 at 09:24
  • 31
    If, for example, you have a exclusion, you will get with a sporadic bug that is difficult to track. A better way to achieve the next/previous would be, something like: Post.where("id > ?", id).limit(1) for next and Post.where("id < ?", id).limit(1) to get the previous – Gabriel Mazetto Jan 09 '12 at 03:44
  • 14
    And if a `Post` gets deleted from the db? – Nick Mar 31 '14 at 15:08
  • 1
    The described use case might be bad (see the other comments), but the essence "it's actually pretty useful to have the choice of whether an exception is raised or not" is totally correct, IMHO! – David Oct 23 '18 at 09:27