:throw option for AR finders that don't throw RecordNotFound exceptions

rubyonrails-core
1

When using ActiveRecord finders that don't by throw a RecordNotFound exception I'm finding myself writing code like the following:

   def show      @tag = Tag.find_by_name(params[:id]) or raise ActiveRecord::RecordNotFound    end

I don't really like the idea of raising a RecordNotFound myself, as, at the least, I don't get the exception message that ActiveRecord includes if it throws the exception internally ("Couldn't find Tag with ID=9999")... and it just feels... wrong.

Wouldn't it be better to have a finder option to specify you want an exception thrown if there's no record found?

   def show      @tag = Tag.find_by_name(params[:id], :throw_not_found => true)    end

Other possible APIs:

:throw_if_not_found => true :throw_if_nil => true :throw_when_not_found => true :throw_when_nil => true :throw_on_not_found => true :throw_on_nil => true :not_found => :throw

Thoughts?

-- tim lucas

2

Tim,

I like the idea. I'd suggest :must_exist => true as another possible API.

- Jamis

Tim Lucas wrote:

3

Isn't find_by_name handled in method missing? Could we add a check for a bang at the end so:

@tag.find_by_name # => doesn't throw if nil @tag.find_by_name! # => throws with nil

PDI

Kev

4

Isn't find_by_name handled in method missing? Could we add a check for a bang at the end so:

@tag.find_by_name # => doesn't throw if nil @tag.find_by_name! # => throws with nil

PDI

I prefer the bang approach to the :some_hash_key thing. In the past I've wanted:

Whatever.find(:one, ...) which is like find first but throws for <> 1 rows.... But this seems like a nice middle ground.

5

I knew somebody would have a better suggestion :slight_smile:

-- tim

6

I’d prefer a bang! method. I think it’s a good indication that there should be separate ‘loading’ and ‘finding’ API – passing ever more options to find is getting weary.

Loading a unique record from the database is semantically different than finding a record that meets some criteria and it could be treated as such.

jeremy

7

You'd then also have to add a find! and document this in both the dynamic finders section of the docs as well as the find! method.

Seems simpler to add it as an option, but i do like the bang. hrmm...

Also, with a namespace of 1, could the bang possibly be needed for another purpose in the future? Nothing really comes to mind.

-- tim

8

You'd then also have to add a find! and document this in both the dynamic finders section of the docs as well as the find! method.

def find!(*args)   val = find(*args)   raise blah if val.nil? or (val == )   return val end

Or something like that. I wrote it in gmail so it isn't tested.

9

You'd then also have to add a find! and document this in both the dynamic finders section of the docs as well as the find! method.

def find!(*args)   val = find(*args)   raise blah if val.nil? or (val == )

val.blank?, man :wink:

10

Does .blank? handle as well as "" and nil? I was going to use .empty? but then I'd have to check that the method existed.

11

blank? handles

class Array #:nodoc:   alias_method :blank?, :empty? end

12

From the source comments actually, so {} and " " too...

# "", " ", nil, , and {} are blank

13

Oh yes, it's ingeniuos. See http://dev.rubyonrails.org/browser/trunk/activesupport/lib/active_support/core_ext/blank.rb

//jarkko

14

wonderous. Either way, easy to write find!

15

blank? handles

class Array #:nodoc:   alias_method :blank?, :empty? end

.blank?

=> true

nil.blank?

=> true

"".blank?

=> true

16

module ActiveRecord    class Base      def find!(*args)        records = find(args)        raise RecordNotFound, "Couldn't find #{name} without an ID" if records.blank?        records      end    end end

works a treat. Adding ! to the method_missing finders might be a bit trickier.

I've filed a ticket seeing as Aksimet seems to be in a good mood at the moment: http://dev.rubyonrails.org/ticket/5974

-- tim

17

Akismet is disabled due to its patch gobbling.

jeremy

18

So, if this `find!' bidness gets introduced it means an exception can get raised a) on record not found via find! / find_by_*! or b) on record not found via find(id). Raised on bangs or on non-bang.

I know the non-bang is a very specific circumstance, but does this bother anyone else? I suppose one way of looking at this is that the bang methods are just sugar for existing methods, none of which change. But it sure would be nice to just say "methods with a bang raise an exception on blank, methods without do not."

I can deal with it; I love that this change is being discussed. But won't someone think of the children?

19

+1 to Chris.

I often override find(id) to return nil instead of raising. I would love to see find() return ||= nil and find!() return ||= raise.

Steven A Bristol http://stevenbristol.blogspot.com

20

-1

I count on the fact that find(id) raises an exception. If I'm looking up a model by id and it's not there, there's usually a problem. The raises RecordNotFound exception is a convenient hook to render a 404 page.

If you really want it to return nil, why not use find_by_id ?