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
Tim Lucas wrote: > 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
> 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
On 8/29/06, Tim Lucas <t.lu...@toolmantim.com> wrote:
> Wouldn't it be better to have a finder option to specify you want an > exception thrown if there's no record found?
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.
> --~--~---------~--~----~------------~-------~--~----~ > You received this message because you are subscribed to the Google > Groups "Ruby on Rails: Core" group. > To post to this group, send email to rubyonrails-core@googlegroups.com > To unsubscribe from this group, send email to rubyonrails-core- > unsubscribe@googlegroups.com > For more options, visit this group at http://groups.google.com/ > group/rubyonrails-core > -~----------~----~----~----~------~----~------~--~---
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.
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?
> 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?
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 ?
On 8/30/06, Chris Wanstrath <ch...@ozmm.org> wrote:
> 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?
I remarked earlier that I think it's a good indication that there should be separate 'loading' and 'finding' API. Find is way overloaded, though that can be valuable trait, as with Ruby's relatively small set of builtin collection classes each serving many needs.
Perhaps: Person.load(1) Person.load_by_name('bob') meaning "give me the record that I've uniquely identified for you" versus Person.find_by_name('bob') meaning "look for records matching the criteria I've given and return the first one"
> 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 ?
> 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 ?
On Aug 30, 2006, at 12:39 PM, Zack Chandler wrote:
> On 8/30/06, Rick Olson <technowee...@gmail.com> wrote: >> If you really want it to return nil, why not use find_by_id ?
> I agree with Rick here. I count on find(id) throwing an exception. I > use find_by_id(id) if I want it the other way.
I agree with you guys, all I'm saying is that if we go ahead with find! we will have a `find' which does about a million different things. It sometimes raises exceptions if there's a bang and sometimes raises exceptions if there's not a bang. Complexity?
I like what Jeremy is saying about load vs find. I think that's what I'm reaching for. When you find(13) you're asking Rails to load a specific record. When you find_by_id(13) you're like "hey give me this if you can, if not that's cool too." Maybe that is a route to discuss.
I think maybe the issue is that the behavior of find_by_id(id) and find(id) is un-least-surprising.
I would be very happy if bang-find always threw an exception on empty, no matter the parameters or dynamic inlined fields in the method name, and no-bang-find never did. We already use ! for that purpose on create and save as had been noted so I don't think it would make the API less Ruby-esque.
|
