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 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.
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.
Yep. I brought up the point of find() being way overloaded back in Feb,
and got no traction. The current find() API is about 3 different APIs
rolled into one method. When not finding a record, find will either throw
an exception, return nil, or return an empty array, depending on if it was
called by id, :first, or :all. Having the method parse its params to
determine how to handle things is ugly. Yes, it's the same as
render(:whatever), but it's still ugly, moreso since the id case can
accept several kinds of values (int, list, array). Adding a find! and
find_by_whatever! and find_or_create_by_whatever! might have advantages in
places, but if we're really going there, can we maybe come up with a big
picture of where we want to be long-term and then figure out how to get
there in a way that's not too painful?
load() vs find() could be a good start, but it's obviously a 2.0 kind of
change since it is not backward compatible. My vote would include
separating the find(id), find(:first) and find(:all) cases into separate
methods. I know find_first and find_all are deprecated (for good reason -
the positional parameter API sucked), but perhaps something like find_one
and find_many would work.
I know find_first and find_all are deprecated (for good reason -
the positional parameter API sucked), but perhaps something like find_one
and find_many would work.
Because find(:first) will actually bring you the first record of the result set. If you include :conditions and :order, then you have a restricted set with a specific order. Then :first makes sense.
Although find(:one) would be awesome for doing a random – “bring me a random record from this result set”
Actually, if you search around ruby-talk the bang methods can just be
used more generally to mean a "dangerous operation". At least this is
what I found in a thread from 04:
It seems to me that :first only makes sense if you add an :order option, because then there is a record that can logically be first. But when you use it on its own it seems to imply some sort of order that doesn’t actually exist.
When I’ve been teaching Rails I’ve often found that people expect find(:first) to return the record with the lowest id number, which is often not the case. find(:one) would probably remove that bit of confusion.
But I agreee that once you add an :order clause, find(:first, :order => … ) makes more sense :).
Another thought .... how about replacing find(:first) with find(:one). :first implies some sort of order where none actually exists.
Not replacing, but creating a new option. find :first would find only
the first record; if there are more than one, only the first is
returned. find :one would imply that one and only one match should
exist: if no match is found, RecordNotFound would be raised; if more
than one match is found, something like NonUniqueResult would be raised.
There’s a few examples in Rails core for this exact sort of behaviour:
ActiveRecord::Base.create vs. ActiveRecord::Base.create!
If create fails it returns an unsaved record. If create! fails you end up with a raised exception.
Base.create! calls Base.save! which, again, raises an exception instead of returning false on failure.
To me the bang on a method that is to perform the same actions as the non-bang method would mean “if this fails, you MUST handle it” instead of the non-bang method which fails silently with only a nil/false return value.
I also second the idea of a some sort of api split between loads and finds but it certainly needs refinement. I think an api change like this early on is better than a confusing or overloaded api that eventually drives people away from the framework.
