no sql in the controller guideline

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

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

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

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.

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

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.

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:

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