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
--~--~---------~--~----~------------~-------~--~----~
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?hl=en
-~----------~----~----~----~------~----~------~--~---
I may be a little biased (I just submitted a patch related to method_missing
and dynamic finders - 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
On 8/20/07, Courtenay <court3nay@gmail.com> wrote:>
>
> 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
>
> --~--~---------~--~----~------------~-------~--~----~
> 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?hl=en
> -~----------~----~----~----~-- ----~----~------~--~---
>
>
--~--~---------~--~----~------------~-------~--~----~
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?hl=en
-~----------~----~----~----~------~----~------~--~---
On 8/21/07, Ben Scofield <bscofield@gmail.com> wrote:> 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...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? -- * J * ~ --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---
> 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... -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---
On 8/21/07, Michael Koziarski <michael@koziarski.com> wrote:> 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 :) -- * J * ~ --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---
On 8/21/07, James Adam <james.adam@gmail.com> wrote:> > On 8/21/07, Michael Koziarski <michael@koziarski.com> wrote: > > 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". --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---
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. -- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---
On 8/21/07, Courtenay <court3nay@gmail.com> wrote:> > ... > > 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".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). --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---
On 8/21/07, Pratik <pratiknaik@gmail.com> wrote:> > > 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.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. --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---
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. -- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---
On Aug 21, 2007, at 11:39, James Adam wrote:> My point was really that a couple of Rdoc-umentable methods might be > better than a swathe of dynamically-generated ones.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 --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---
On 8/21/07, Mislav Marohnić <mislav.marohnic@gmail.com> wrote:> On 8/21/07, Pratik <pratiknaik@gmail.com> wrote: > > > > 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. > > 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?Yeah I was curious more than anything else :) Magic 1, Defined 0 --~--~---------~--~----~------------~-------~--~----~ 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?hl=en -~----------~----~----~----~------~----~------~--~---