重構此控制器？

Question

class ArticlesController < ApplicationController 

    def index 
    @articles = Article.by_popularity 

    if params[:category] == 'popular' 
     @articles = @articles.by_popularity 
    end 

    if params[:category] == 'recent' 
     @articles = @articles.by_recent 
    end 

    if params[:category] == 'local' 
     index_by_local and return 
    end 

    if params[:genre] 
     index_by_genre and return 
    end 

    respond_to do |format| 
     format.html # index.html.erb 
     format.xml { render :xml => @articles } 
    end 
    end 

    def index_by_local 
    # 10 lines of code here 

    render :template => 'articles/index_by_local' 
    end 

    def index_by_genre 
    # ANOTHER 10 lines of code here 

    render :template => 'articles/index_by_genre' 
    end 
end 

正如你可以從上面看到的。我的控制器不完全是薄。它所做的是取決於傳遞的參數，它與模型交互以過濾記錄。如果params[:local]或params[:genre]通過了，然後分別調用它自己的方法（def index_by_local和def index_by_genre）做進一步處理。這些方法也加載自己的模板，而不是index.html.erb。

這對控制器來說很典型嗎？或者我應該重構這個不知何故？

Answer 1

我們可以移動的前幾行到模型（article.rb）：

def get_by_category(category) 
    # Return articles based on the category. 
end 

這樣，我們完全可以測試文章使用單元測試取邏輯。

通常移動所有與在模型中獲取記錄相關的代碼。 控制器一般

1. 應授權用戶
2. GET記錄使用PARAMS並將它們分配到實例變量 [這些通常必須是功能 調用模型]
3. 渲染或重定向

Answer 2

我會爲您想要使用的每個集合定義範圍。

class Article < ActiveRecord::Base 
    ... 
    scope :popular, where("articles.popular = ?", true) # or whatever you need 
    scope :recent, where(...) 
    scope :by_genre, where(...) 
    scope :local, where(...) 
    ... 

    def self.filtered(filter) 
    case filter 
    when 'popular' 
     Article.popular, 'articles/index' 
    when 'recent' 
     Article.recent, 'articles/index' 
    when 'genre' 
     Article.by_genre, 'articles/index_by_genre' 
    when 'local' 
     Article.local, 'articles/index_by_local' 
    else 
     raise "Unknown Filter" 
    end 
    end 
end 

然後在你的控制器動作，這樣的事情：

def index 
    @articles, template = Article.filtered(params[:category] || params[:genre]) 
    respond_to do |format| 
    format.html { render :template => template } 
    format.xml { render :xml => @articles } 
    end 
end