[转]Skinny Controller, Fat Model

不错的文章，出处：http://weblog.jamisbuck.org/2006/10/18/skinny-controller-fat-model
When first getting started with Rails, it is tempting to shove lots of logic in the view. I’ll admit that I was guilty of writing more than one template like the following during my Rails novitiate:

<!-- app/views/people/index.rhtml -->
<% people = Person.find(
      :conditions => ["added_at > ? and deleted = ?", Time.now.utc, false],
      :order => "last_name, first_name") %>
<% people.reject { |p| p.address.nil? }.each do |person| %>
  <div id="person-<%= person.new_record? ? "new" : person.id %>">
    <span class="name">
      <%= person.last_name %>, <%= person.first_name %>
    </span>
    <span class="age">
      <%= (Date.today - person.birthdate) / 365 %>
    </span>
  </div>
<% end %>

Not only is the above difficult to read (just you try and find the HTML elements in it), it also completely bypasses the “C” in “MVC”. Consider the controller and model implementations that support that view:

# app/controllers/people_controller.rb
class PeopleController < ActionController::Base
end

# app/models/person.rb
class Person < ActiveRecord::Base
  has_one :address
end

Just look at that! Is it really any wonder that it is so tempting for novices to take this approach? They’ve got all their code in one place, and they don’t have to go switching between files to follow the logic of their program. Also, they can pretend that they haven’t actually written any Ruby code; I mean, look, it’s just the template, right?

For various reasons, though, this is a very, very bad idea. MVC has been successful for many reasons, and some of those reasons are “readability”, “maintainability”, “modularity”, and “separation of concerns”. You’d like your code to have those properties, right?

A better way is to move as much of the logic as possible into the controller. Seriously, isn’t that what the controller is for? It is supposed to mediate between the view and the model. Let’s make it earn its right to occupy a position in our source tree:

<!-- app/views/people/index.rhtml -->
<% @people.each do |person| %>
  <div id="person-<%= person.new_record? ? "new" : person.id %>">
    <span class="name">
      <%= person.last_name %>, <%= person.first_name %>
    </span>
    <span class="age">
      <%= (Date.today - person.birthdate) / 365 %>
    </span>
  </div>
<% end %>

# app/controllers/people_controller.rb
class PeopleController < ActionController::Base
  def index
    @people = Person.find(
      :conditions => ["added_at > ? and deleted = ?", Time.now.utc, false],
      :order => "last_name, first_name")
    @people = @people.reject { |p| p.address.nil? }
  end
end

Better! Definitely better. We dropped that big noisy chunk at the top of the template, and it’s more immediately obvious what the structure of the HTML file is. Also, you can see by reading the controller code roughly what kind of data is going to be displayed.

However, we can do better. There’s still a lot of noise in the view, mostly related to conditions and computations on the model objects. Let’s pull some of that into the model:

# app/models/person.rb
class Person < ActiveRecord::Base
  # ...

  def name
    "#{last_name}, #{first_name}"
  end

  def age
    (Date.today - person.birthdate) / 365
  end

  def pseudo_id
    new_record? ? "new" : id
  end
end

<!-- app/views/people/index.rhtml -->
<% @people.each do |person| %>
  <div id="person-<%= person.pseudo_id %>">
    <span class="name"><%= person.name %></span>
    <span class="age"><%= person.age %></span>
  </div>
<% end %>

Wow. Stunning, isn’t it? The template is reduced to almost pure HTML, with only a loop and some simple insertions sprinkled about. Note, though, that this is not just a cosmetic refactoring: by moving name, age and pseudo_id into the model, we’ve made it much easier to be consistent between our views, since any time we need to display a person’s name or age we can simply call those methods and have them computed identically every time. Even better, if we should change our minds and decide that (e.g.) age needs to be computed differently, there is now only one place in our code that needs to change.

However, there’s still a fair bit of noise in the controller. I mean, look at that index action. If you were new to the application, coming in to add a new feature or fix a bug, that’s a lot of line noise to parse just to figure out what is going on. If we abstract that code into the model, we can not only slim the controller down, but we can effectively document the operation we’re doing by naming the method in the model appropriately. Behold:

# app/models/person.rb
class Person < ActiveRecord::Base
  def self.find_recent
    people = find(
      :conditions => ["added_at > ? and deleted = ?", Time.now.utc, false],
      :order => "last_name, first_name")
    people.reject { |p| p.address.nil? }
  end

  # ...
end

# app/controllers/people_controller.rb
class PeopleController < ActionController::Base
  def index
    @people = Person.find_recent
  end
end

Voila! Looking at PeopleController#index, you can now see immediately what is going on. Furthermore, in the model, that query is now self-documenting, because we gave the method a descriptive name, find_recent. (If you wanted, you could even take this a step further and override the find method itself, as I described in Helping ActiveRecord finders help you. Then you could do something like Person.find(:recent) instead of Person.find_recent. There’s not a big advantage in that approach in this example, so it mostly depends on what you prefer, esthetically.)

Be aggressive! Try to keep your controller actions and views as slim as possible. A one-line action is a thing of wonder, as is a template that is mostly HTML. It is also much more maintainable than a view that is full of assignment statements and chained method calls.

Another (lesser) nice side-effect of lean controllers: it allows respond_to to stand out that much more, making it simple to see at a glace what the possible output types are:

# app/controllers/people_controller.rb
class PeopleController < ActionController::Base
  def index
    @people = Person.find_recent

    respond_to do |format|
      format.html
      format.xml { render :xml => @people.to_xml(:root => "people") }
      format.rss { render :action => "index.rxml" }
    end
  end
end

Give all this a try in your next project. Like adopting RESTful practices, it may take some time to wrap your mind around the refactoring process, especially if you’re still accustomed to throwing lots of logic in the view. Just be careful not to go too far; don’t go putting actual view logic in your model. If you find your model rendering templates or returning HTML or Javascript, you’ve refactored further than you should. In that case, you should make use of the helper modules that script/generate so kindly stubs out for you in app/helpers. Alternatively, you could look into using a presenter object.