重構類似的控制器的動作

我用拉審查審查我的應用程序的代碼，並將其帶回來這樣的：重構類似的控制器的動作

考慮檢測重構，類似的代碼。

發生在：

SkillsController＃索引

PagesController＃索引

所以應用程序/控制器/ skills_controller.rb索引行動碼是：

def index 
    @skill = Skill.new 
    if params[:search] 
    @skills = Skill.search(params[:search]).order('created_at DESC') 
    else 
    @skills = Skill.all.order('created_at DESC') 
    end 
end

和上app/controllers/pages_controller.rb是：

def index 
    @users = User.all 
    if params[:search] 
    @users = User.search(params[:search]).order('created_at DESC') 
    else 
    @users = User.all.order('created_at DESC') 
    end 
end

我想以某種方式重構這兩個控制器上的這兩個動作？另外，我不確定我是如何重構這個的。我是否抽取if params [：search]段並將實例變量替換爲將在這兩個操作中使用的另一個變量？

謝謝你的時間。

來源

2014-03-01 Stefanos.Ioannou

全球它告訴你，這兩種方法非常相似，你可能會提取一些常見的代碼;在附註中，第一眼看起來你的代碼有一個錯誤，用@users = Skill.all.order（'created_at DESC'）'應該可能是'@users = User.all.order（'created_at DESC'）'，這可能會鼓勵你的工具認爲它是重複的代碼。（注意：我完全不知道Ruby語法） – Gorkk

感謝您注意到錯誤 –

要注意的是，第二個代碼示例中的@users = User.all行看起來沒有必要 - 不知道它看起來的語法一個變量賦值 - 考慮到if和else兩個部分都有一個@users = ...語句（但這可能只是因爲我不知道語法而遺漏了一些東西） - @skill =第一部分中的Skill.new'似乎也不太適用。 – Gorkk

我不知道你的方法search來自哪裏。它似乎來自ActiveRecord的自定義模塊/寶石。

如果是這樣，你可以在控制器改變方法來縮短控制器

def self.search(args) 
    return self unless args 
    original_search_logic args 
end 

# As well as extract order to a scope 
scope :by_time, -> { order('created_at DESC') }

代碼然後：

# Skill 
def index 
    @skills = Skill.search(params[:search]).by_time 
end 

# User 
def index 
    @users = User.search(params[:search]).by_time 
end

這些應該現在有足夠的乾燥。

來源

2014-03-01 16:43:41

看看has_scope和inherited_resources。你可以用has_scope提取params [：search]部分。並使用inherited_resources來提取如何獲取集合並執行排序。

來源

2014-03-01 14:15:29 nickcen

重構類似的控制器的動作

回答

相關問題