Forum: Ruby on Rails no sql in the controller guideline

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Py J. (Guest)
on 2009-05-05 23:33
hello. i just checked Chad F.'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-develo...
Tyler MacDonald (Guest)
on 2009-05-05 23:44
(Received via mailing list)
PP Junty <removed_email_address@domain.invalid> wrote:
>
> 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
Charles J. (Guest)
on 2009-05-05 23:44
(Received via mailing list)
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
Py J. (Guest)
on 2009-05-06 02:52
Charles J. 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.
Frederick C. (Guest)
on 2009-05-06 03:32
(Received via mailing list)
On May 5, 11:52 pm, PP Junty <removed_email_address@domain.invalid> wrote:
> Charles J. 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
Py J. (Guest)
on 2009-05-06 04:14
> 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 good

i 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):
> good

i'll check this strategy, thanks.
Phlip (Guest)
on 2009-05-06 05:20
(Received via mailing list)
PP Junty wrote:

> hello. i just checked Chad F.'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 (Guest)
on 2009-05-06 06:38
(Received via mailing list)
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
Py J. (Guest)
on 2009-05-07 02:34
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.
Julian L. (Guest)
on 2009-05-07 06:05
(Received via mailing list)
On 06/05/2009, at 12:37 PM, Phlip <removed_email_address@domain.invalid> wrote:

>
> 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
This topic is locked and can not be replied to.