How to make the view simpler, the controller more useful?

Question

This question relates to cleaning up the view and giving the controller more of the work.

I have many cases in my project where I have nested variables being displayed in my view. For example:

# controller
@customers = Customer.find_all_by_active(true)
render :layout => 'forms'

# view
<% @customers.each do |c| %>
  <%= c.name %>
  <% @orders = c.orders %>  # I often end up defining nested variables inside the view
    <% @orders.each do |o| %>
    ...
    <% end %>
<% end %>

I am fairly new to RoR but it seems that what I'm doing here is at odds with the 'intelligent controller, dumb view' mentality. Where I have many customers, each with many orders, how can I define these variables properly inside my controller and then access them inside the view?

If you could provide an example of how the controller would look and then how I would relate to that in the view it would be incredibly helpful. Thank you very much!

Answer 1

I don't think there is anything drastically wrong with what you're doing. Looping through the customers and outputting some of their attributes and for each customer, looping through their orders and outputting some attributes is very much a view-oriented operation.

In the MVC architecture, the controller has responsibility for interacting with the model, selecting the view and (certainly in the case of Rails) providing the view with the information it needs to render the model.

You might consider extracting the code into a view helper though, if you have that exact code repeated more than once. You could even genericize it, passing in the name of a model and association. I haven't tested it, but you should be able to do something like this:

def display_attributes(models, association, attribute, association_attribute)
  content = ''
  models.each do |m|
    content << "<p>#{m.attribute}</p>"
    associated_models = m.association
    associated_models.each do |am|
      content << "<p>#{am.association_attribute}</p>"
    end
  end
  content
end

Then in the view, you could use the helper like this:

<%= display_attributes(@customers, orders, name, name) %>

Obviously you would change the HTML markup within the helper method to suit your requirements. Note that if you're not using Rails 3 then you'll want to escape the output of the attribute names in the helper method.

Answer 2

I don't think there's anything wrong with your code. I'd just suggest for you to use a :include in your find

@customers = Customer.find_all_by_active(true, :include => :orders)

to reduce the number of queries.

Answer 3

I see nothing wrong with the code as you showed.

You are mixed up about the "intelligent controller, dumb view" approach though, i tend to prefer the "skinny controller, fat model", so indeed the view should be dumb, but you put the intelligence inside your model, and your helpers (or use a presenter), but definitely not in the controller.