Ever since the ActiveRecord::Base.first and last convenience methods were added, I've wanted to be able to call them as they can be called on an array, passing a limit, n, as the first argument.
Person.first(5) is a bit sweeter than Person.all(:limit => 5).
Ticket/patch here:
Any thoughts?
Stephen
I’m not sure. AR::Base subclasses don’t act like Arrays, so I don’t think we should be matching same-named method implementations just for the sake of doing so, regardless of how sweet the syntactic sugar is.
Well, I do think that it goes a little beyond syntactic sugar. AR::Base subclasses don't act like arrays, but their scopes and association proxies do, to an extent. Consider a named scope, the behavior through Ruby becomes more clear.
Article.published.first(5) # Selects without a limit unless there is a limit on the named scope.
It actually looks like the patch needs a little adjusting to fix this case for named scopes, but if we're to make scopes smarter about this array method, it only makes sense, for consistency, to offer the same behavior at the AR::Base class level.
I've wanted to use #first and #last the same way, and in fact keep forgetting that I can't. I don't see any downside to supporting the syntax.
Cheers, Luke
Does Person.first(1) return an collection or an object?
It would return a collection in order to remain consistent with association proxies, scopes, and the arrays they act like (especially after they've already executed).
Person.active.first(1) # => [#<Person>] Person.active.first # => #<Person> Person.first(1) # => [#<Person>] Person.first # => #<Person> Person.find(1).blog_posts.first(1) # => [#<BlogPost>] Person.find(1).blog_posts.first # => #<BlogPost>
The #first and #last class methods on ActiveRecord::Base are examples of Rails cleverness gone a little over the edge. Who hasn't run into this problem?:
Person.blog_posts.first # => #<BlogPost name='foo'> Person.blog_posts.first.name = 'bar' Person.blog_posts.first # => #<BlogPost name='foo'>
The #first and #last methods always go to the database when called on an unloaded association collection proxy, and do not update the association target with the resulting record. So:
Person.blog_posts.loaded? # => false Person.blog_posts.first # => #<BlogPost> Person.blog_posts.loaded? # => false Person.blog_posts.target # =>
This is obviously for performance reasons, given the assumption that the most frequent case will be someone loading the first or last record once, in which case returning a single record from the database is faster than returning all records and then asking the underlying array for the first one. Of course, in other cases, this causes problems. For instance:
Person.blog_posts.count # => 5 Person.blog_posts.first.mark_for_destruction # => true Person.save! # => true Person.blog_posts.count #= 5 (!)
Now, if you start returning an arbitrarily large number of records from the #first and #last methods, the single advantage of those methods (performance) goes away, leaving only liabilities. Consider this:
Person.blog_posts(100) # => [#<BlogPost, #<BlogPost]... ] Person.blog_posts # => [#<BlogPost, #<BlogPost]... ]
Both of those calls generated database calls; the first returned 100 records, the second returned all records *including the 100 already returned previously*. And nothing in that code makes it clear that you're doing more database calls than necessary, or that you're creating two in-memory objects for each of the 100 records returned twice, or that the first 100 records aren't included in the association proxy target.
Note that calling #first or #last with a count parameter on a collection proxy works as expected with the current code because it loads records and then delegates to the underlying array. This patch would modify this existing behavior on association collections.
I realize the original intention was to add this capability to ActiveRecord::Base class objects, but keep in mind you can't add an ActiveRecord::Base class method without affecting the way association proxies work (consider #sum).
The #first and #last class methods on ActiveRecord::Base are examples of Rails cleverness gone a little over the edge.
The cleverness may go a little over the edge, but, to add to your list of general confusion with these inconsistencies:
Person.scoped({}).first # SELECT * FROM `people` LIMIT 1
=> #<Person>
Person.scoped({}).first(5) # SELECT * FROM `people`
=> [#<Person>, #<Person>, #<Person>, #<Person>, #<Person>]
It works the same way with association proxies. Call "first" or "last" with an number argument and Rails doesn't optimize, but call "first" or "last" with an options hash or no argument, and it does. This inconsistent behavior could easily cause confusion.
To add one more layer of inconsistency:
Person.first(5) # SELECT * FROM `people` LIMIT 1
=> #<Person>
Now, if you start returning an arbitrarily large number of records from the #first and #last methods, the single advantage of those methods (performance) goes away, leaving only liabilities.
Well, even an exaggerated example like @site.users.first(100) would offer much better performance if that integer limit were respected, if there were hundreds of thousands of users.
Note that calling #first or #last with a count parameter on a collection proxy works as expected with the current code because it loads records and then delegates to the underlying array. This patch would modify this existing behavior on association collections.
Technically, this patch doesn't (as it stands) modify this behavior, because named scopes and association proxies type check the first argument for integers before performing.
I realize the original intention was to add this capability to ActiveRecord::Base class objects, but keep in mind you can't add an ActiveRecord::Base class method without affecting the way association proxies work (consider #sum).
Actually, I did intend to add the capability through scopes and proxies, as well, if there was community interest.
I do think seeking some kind of consistency is desirable. Making the convenience methods cleverer just seems to be most backwards- compatible way of handling it, retaining backwards compatibility and general performance benefits.
Stephen
That's true. However, has_many associations and named scopes do act like Arrays for the most part, and we implement most of that goodness on AR::Base.
I would say that your argument is a fair one, but by that argument we should not have .first and .last at all.
We do have them, a lot of us find them very convenient, and a number of us would find the form with the numeric argument convenient too - there were a lot of people keen on #2591 Add magic encoding comment to generated files - Ruby on Rails - rails, which is for the same issue, and the only arguments against were people who wanted more (file yer own ticket! ;)) and one person who doesn't want to use this (you don't have to).
Almost all of ActiveRecord is syntatic sugar for things you could do the hard way if you want to. Where we have added sugar, we should make it consistent with the rest of the language and framework, when it is useful to do so - and in this case it clearly is, given the number of people who are interested in this simple consistency extension.
If this doesn't gain any traction, or till it does, I've fashioned an opinionated little gem wherein I'll keep basic patches I tend to use:
http://github.com/stephencelis/rerails
Hopefully others can get some use out of them, too.
Stephen
Keep up to date with Rails on Twitter and This Week in Rails
Policies: Conduct, License, Maintenance, Security, Trademarks