What do you all think of this syntax:
User.find_by(:name, "Joe")
instead of
User.find_by_name("joe")
thus removing at least part of the method_missing hack, and also, removing a little of the magic "What is this find_by_stuff and why can't I see it in the API?"
I hear that it'll speed things up and the stacktraces will be simpler.
Apologies if this has been covered previously.
Courtenay
I may be a little biased (I just submitted a patch related to method_missing and dynamic finders - [
http://dev.rubyonrails.org/ticket/9317](http://dev.rubyonrails.org/ticket/9317)), but I think such a change would be ill-advised. For one thing, I can’t think of a version of this syntax that would look acceptable when you’re checking multiple attributes:
User.find_by([:name, :age], [“Joe”, 21]) # just… no
User.find_by(:name => “Joe”, :age => 21) # better, but then why not just use:
User.find(:all, :conditions => {:name => “Joe”, :age => 21})
I don’t know that the hash version is noticeably easier than passing a hash to :conditions…
As for performance - performance doesn’t appear to suffer too much with the method missing implementation: http://m.onkey.org/2007/8/18/ar-dynamic-finders-are-soooo-slow-not (and with my aforementioned patch, it’s even less of an issue).
I can’t speak to the stack trace, since I haven’t futzed around with it anyway to check just how much it would change…
Ben
I would imagine that User.find_by(conditions_hash) is probably a common enough usage that it's worth wrapping in a method, even if that method's implementation is only something like:
def find_by(conditions_hash, *args) find(:all, :conditions => conditions_hash, *args) end
Certainly all you're saving is around ten keystrokes, but there's a clarity benefit, and doesn't require any significant effort to understand.
The optimisations Ben's patch provides are certainly great from a performance point of view, but I think that Court3nay's argument still stand; the one thing that springs to mind when I think about dynamically-generated methods in Rails is the nightmare of debugging routes issues - surely we should be trying to remove as much magic as possible that isn't necessary?
Certainly all you're saving is around ten keystrokes, but there's a clarity benefit, and doesn't require any significant effort to understand.
It does, however, add a whole other method to our supported api. Every feature we add has to be supported in future releases. Saving 10 keystrokes isn't quite enough to get over that hump. There are some unanswered questions though:
* How would it work with options like :order, :include * Does it return :first or :all? the name implies :first but your proposed implementation is :all.
The optimisations Ben's patch provides are certainly great from a performance point of view, but I think that Court3nay's argument still stand; the one thing that springs to mind when I think about dynamically-generated methods in Rails is the nightmare of debugging routes issues - surely we should be trying to remove as much magic as possible that isn't necessary?
I hardly think that Site.find_by_domain(request.host) is anywhere near as magic as some of the edge cases in routes. I think it's pretty neat, on the other hand:
User.find_all_by_email_and_subscription_status_and_locked(email, 'premium', false)
Is something of an abomination...
It does, however, add a whole other method to our supported api. Every feature we add has to be supported in future releases. Saving 10 keystrokes isn't quite enough to get over that hump.
My point was really that a couple of Rdoc-umentable methods might be better than a swathe of dynamically-generated ones. It's an interesting question - is User.find_by_name part of the supported API? It is... but only when the users table has a name column.
There are some unanswered questions though:
* How would it work with options like :order, :include * Does it return :first or :all? the name implies :first but your proposed implementation is :all.
The implementation specifics are certainly important, but distract from the idea I was trying to put forward, which is to use concrete methods that can be documented in code than dynamically generating them, except where there are substantial benefits (such as the attribute accessors, for example).
I hardly think that Site.find_by_domain(request.host) is anywhere near as magic as some of the edge cases in routes.
That's fair - it was a relatively cheap shot, designed only to
reinforce the above 
> It does, however, add a whole other method to our supported api. > Every feature we add has to be supported in future releases. Saving > 10 keystrokes isn't quite enough to get over that hump.
That, or the continuing question "where did this method come from?"
My point was really that a couple of Rdoc-umentable methods might be better than a swathe of dynamically-generated ones. It's an interesting question - is User.find_by_name part of the supported API? It is... but only when the users table has a name column.
> There are some unanswered questions though: > > * How would it work with options like :order, :include > * Does it return :first or :all? the name implies :first but your > proposed implementation is :all.
The implementation specifics are certainly important, but distract from the idea I was trying to put forward, which is to use concrete methods that can be documented in code than dynamically generating them, except where there are substantial benefits (such as the attribute accessors, for example).
That's where I was headed. Yay!
> I hardly think that Site.find_by_domain(request.host) is anywhere near > as magic as some of the edge cases in routes.
Which do you prefer?
Site.find_by_domain(request.host) Site.find_by(:domain, request.host)
The former has less symbols, the latter is more searchable-in-the-API and while it only covers a simple use-case, I feel it's "better".
And what about find_or_(initialize|create)_by_what_and_ever ?
Even though I like Site.find_by(:domain, request.host) more that Site.find_by_domain, it seems like we're solving the wrong problem.
Are people really getting confused by the dynamic finders not being in the API? It’s my impression that they are 1) pretty noticeable and identifiable, since they all follow the same pattern, and 2) cool for Rails newbies, so most people try them out and appear to understand how they work (even if they don’t understand how they’re implemented).
I like find_by_domain more (nicer to type and look at) and I also feel that “solving” this is wrong.
You said that the downside of dynamic finders is that they don’t show up in API documentation, and people are confused where the methods got from. Well, this may be true for people who have never read the basic documentation for AR::Base, because the concept of dynamic finders is nicely outlined there. Those who have quickly realize the methods are a no-brainer. What about dynamic getters and setters on models: how to explain to people that their Post instances have “title” and “published_at” properties? Do those appear in API documentation?
When people started using will_paginate more, I’ve noticed more and more people asked “this paginate_by_domain method, where did it come from? where did ‘paginate’ come from?”. This is because the whole will_paginate API is just method_missing. I’ve then written a lot of Rdoc for the plugin explaining the dynamic nature of the API, and that helped. People understood that will_paginate creates a thin, virtual layer over the existing finder API, part of which is also dynamic (find_by_domain). And they understand find_by_foo_and_bar because they have at least glanced at AR::Base docs.
So in short, I don’t believe the “lack” of documentation and the confusion it “causes” justifies deprecating the API.
This must be for Courtenay, not me 
I agree with everything Mislav said.
So in short, I don't believe the "lack" of documentation and the confusion it "causes" justifies deprecating the API.
And that's what I meant when I said "solving the wrong problem". Real solution is to improve docs.
Well, that's easily solved too.
class ActiveRecord # Documentation for the method def find_by___; end undef :find_by___ end
If we agree on the usage of underscores in the method to replace dynamic parts of the method it shouldn't be too hard to find them in rdoc html or ri.
Manfred
Yeah I was curious more than anything else
Magic 1, Defined 0
Keep up to date with Rails on Twitter and This Week in Rails
Policies: Conduct, License, Maintenance, Security, Trademarks