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

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

Tim,

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

- Jamis

Tim Lucas wrote:

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

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.

I knew somebody would have a better suggestion :slight_smile:

-- tim

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

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

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.

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:

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.

blank? handles []

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

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

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

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

//jarkko

wonderous. Either way, easy to write find!

blank? handles []

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

[].blank?

=> true

nil.blank?

=> true

"".blank?

=> true

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

Akismet is disabled due to its patch gobbling.

jeremy

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?

+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

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