hello. i just checked Chad Fowler''s post "20 Rails Development No-No''s" and one guideline caught my attention. it says: "Nothing that looks at all like SQL should go into a controller, view, or helper." it really came as a surprise to me as Rails itself seems to go against such practice by its AR ''conditions'' option, which most of the times contains SQL code. like in this example from Rails Cookbook: Employee.find :all, :conditions => [''hire_date > ? AND first_name = ?'', ''1992'', ''Andrew''] does anyone have an opinion on this? * complete post at: http://www.chadfowler.com/2009/4/1/20-rails-development-no-no-s -- Posted via http://www.ruby-forum.com/.
PP Junty <rails-mailing-list-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org> wrote:> "Nothing that looks at all like SQL should go into a controller, view, > or helper." > > it really came as a surprise to me as Rails itself seems to go against > such practice by its AR ''conditions'' option, which most of the times > contains > SQL code. like in this example from Rails Cookbook: > > Employee.find :all, :conditions => [''hire_date > ? AND first_name = ?'', > ''1992'', ''Andrew''] > > does anyone have an opinion on this?You could argue that :conditions is an attribute to a method on the model -- and if you wanted a lookup like that, you should program it into the model like so, and use the abstraction in your controllers/views; def self.find_after_hire_date_by_first_name(date, first_name) self.find :all, :conditions => [ ''hire_date > ? AND first_name = ?'', date, first_name ] end Cheers, Tyler
Like all good programming rules of thumb there are interesting exceptions. Complicated unions and intersections, especially where the from clause might be a dynamic select, such as might be needed for a report can be very difficult to do without resorting to passing the sql directly to the database with a connection object. Cheers-- Charles On Tue, May 5, 2009 at 2:33 PM, PP Junty <rails-mailing-list-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org>wrote:> > hello. i just checked Chad Fowler''s post "20 Rails Development No-No''s" > and > one guideline caught my attention. it says: > > "Nothing that looks at all like SQL should go into a controller, view, > or helper." > > it really came as a surprise to me as Rails itself seems to go against > such practice by its AR ''conditions'' option, which most of the times > contains > SQL code. like in this example from Rails Cookbook: > > Employee.find :all, :conditions => [''hire_date > ? AND first_name = ?'', > ''1992'', ''Andrew''] > > does anyone have an opinion on this? > > > * complete post at: > http://www.chadfowler.com/2009/4/1/20-rails-development-no-no-s > -- > Posted via http://www.ruby-forum.com/. > > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Charles Johnson wrote:> Like all good programming rules of thumb there are interesting > exceptions.this is exactly where things get confused, because code like the example i provided seems to be the norm, not the exception. -- Posted via http://www.ruby-forum.com/.
On May 5, 11:52 pm, PP Junty <rails-mailing-l...-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org> wrote:> Charles Johnson wrote: > > Like all good programming rules of thumb there are interesting > > exceptions. > > this is exactly where things get confused, because code like the > example i provided seems to be the norm, not the exception. >The code you posted is fine, it just depends where you put it: - in a view: super bad - in the controller: not so good - in a model (or these days probably replaced with a named scope): good Fred> -- > Posted viahttp://www.ruby-forum.com/.
> The code you posted is fine, it just depends where you put it:that is why i named the thread "no sql in the controller". :)> - in a view: super bad > - in the controller: not so goodi agree, i just didn''t see any mention to this guideline in the books i consulted or in the AR source code comments.> - in a model (or these days probably replaced with a named scope): > goodi''ll check this strategy, thanks. -- Posted via http://www.ruby-forum.com/.
PP Junty wrote:> hello. i just checked Chad Fowler''s post "20 Rails Development No-No''s" > and > one guideline caught my attention. it says: > > "Nothing that looks at all like SQL should go into a controller, view, > or helper."Things that "look like" SQL include any kind of query more elaborate than a simple accessor call. For example: - hit a service that provides realtime currency exchange rates - hit a service that provides the local weather report - retweet some popular tweets - calculate the integral of a Zipfs Law partition over a population - calculate 40 + 2 - rip a list of items, then .select and .reject its candidates - parse some XML with XPath and find a data point> it really came as a surprise to me as Rails itself seems to go against > such practice by its AR ''conditions'' option, which most of the times > contains > SQL code. like in this example from Rails Cookbook:Then don''t put it in the controller or view - even if the Book does. There''s a super-easy metric to see if you are obeying this rule: How easily can you test your SQL-like statement? If your test must call a controller with get :action, you are farther from your statement than you should. (But note such a test might call an action and detect that it called the correct model method, so long as such a test case is completely auxiliary to your actual method and its real test!) Similarly, you should not call a view, render a page, rip its details out of HTML (even with assert_xhtml!), and then test that your SQL-like statement worked. Put another way, the Law of Demeter should apply more strictly in the controllers and views! -- Phlip http://flea.sourceforge.net/resume.html
Phlip wrote:>> "Nothing that looks at all like SQL should go into a controller, view, >> or helper." > > Things that "look like" SQL include any kind of query more elaborate than a > simple accessor call.I just read _all_ of Fowler''s statement. Helpers are a gray area. In software design, there is the concept of breaking up low-level coupling by escalating dependencies. If you have a statement that goes A.elaborate_query.b.elaborate_query, then does your query belong inside Model A or B? The cheap answer is "pick one at random and keep coding!" The more elaborate answer is if A was decoupled from B we might not want to couple it, so putting the query into a helper makes more sense. (It''s also quite testable there...;) -- Phlip http://flea.sourceforge.net/resume.html
thanks for the tips Phlip, specially the "How easily can you test your SQL-like statement?" metric. Phlip wrote:>>> "Nothing that looks at all like SQL should go into a controller, view, >>> or helper." > Helpers are a gray area. In software design, there is the concept of > breaking up low-level coupling by escalating dependencies.i think that in this case he is talking about Rails helpers, which are supposed to contain only helper methods used by the views. -- Posted via http://www.ruby-forum.com/.
On 06/05/2009, at 12:37 PM, Phlip <phlip2005-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> > Phlip wrote: > >>> "Nothing that looks at all like SQL should go into a controller, >>> view, >>> or helper." >> >> Things that "look like" SQL include any kind of query more >> elaborate than a >> simple accessor call. > > I just read _all_ of Fowler''s statement. > > Helpers are a gray area. In software design, there is the concept of > breaking up > low-level coupling by escalating dependencies. If you have a > statement that goes > A.elaborate_query.b.elaborate_query, then does your query belong > inside Model A > or B? The cheap answer is "pick one at random and keep coding!" > > The more elaborate answer is if A was decoupled from B we might not > want to > couple it, so putting the query into a helper makes more sense. > (It''s also quite > testable there...;) >I''d disagree. I reckon each query needs to be inside each model. The fact you started with object a means the initial ''full method'' is gonna need to be in object a at least Like... Encapsulation, man. Rails seems to not have enough. It''d be awesome if ActiveRecord encouraged you to build your own model interfaces in the same way the rails community generally disencourages you from using scaffolding. This needs to be a mode. Blog: http://random8.zenunit.com/ Learn: http://sensei.zenunit.com/ Twitter: http://twitter.com/random8r> -- > Phlip > http://flea.sourceforge.net/resume.html > > > >