No sql in the controller guideline


#1

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?


#2

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


#3

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


#4

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.


#5

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


#6

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”. :slight_smile:

  • 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.


#7

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


#8

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…:wink:


Phlip
http://flea.sourceforge.net/resume.html


#9

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.


#10

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…:wink:

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