你將如何整理這個控制器邏輯？

Question

我有一個控制器，設置對象的狀態，一些邏輯，如果某些條件得到滿足：

if params[:concept][:consulted_legal] == 0 && params[:concept][:consulted_marketing] == 1 
    @concept.attributes = {:status => 'Awaiting Compliance Approval'} 
elsif params[:concept][:consulted_marketing] == 0 && params[:concept][:consulted_legal] == 1 
    @concept.attributes = {:status => 'Awaiting Marketing Approval'} 
elsif params[:concept][:consulted_marketing] == 0 && params[:concept][:consulted_legal] == 0 
    @concept.attributes = {:status => 'Awaiting Marketing & Legal Approval'} 
else 
    @concept.attributes = {:status => 'Pending Approval'} 
end 

我之間共享創建和更新操作。你會如何去重構這種惡作劇？尋找最佳實踐。

新手編程和熱衷於清理我的代碼。

TIA。

Answer 1

這是我的承擔。我稱它爲超級乾燥。

statuses = 
    [ 
    ['Awaiting Marketing & Legal Approval','Awaiting Compliance Approval'], 
    ['Awaiting Marketing Approval','Pending Approval'] 
    ] 

{:status => statuses[params[:concept][:consulted_legal].to_i][params[:concept][:consulted_marketing].to_i]} 

另外，一個更傳統的方法 - 具有可讀性但冗長：

status = if params[:concept][:consulted_legal] == "0" 
    if params[:concept][:consulted_marketing] == "1" 
    'Awaiting Compliance Approval' 
    else 
    'Awaiting Marketing & Legal Approval' 
    end 
else 
    if params[:concept][:consulted_marketing] == "0" 
    'Awaiting Marketing Approval' 
    else 
    'Pending Approval' 
    end 
end 

@concept.attributes = {:status => status} 

注意：它看起來像你原來的代碼檢查的複選框值。 params哈希值是總是Strings，而不是Fixnum s所以我的代碼比較字符串。如果由於某種原因比較Fixnum s是您的情況需要什麼，只需拿出數字周圍的引號。

Answer 2

將其分解爲嵌套的if語句。

if params[:concept][:consulted_legal] == '0' 
    if params[:concept][:consulted_marketing] == '1' 
    @concept.attributes = { :status => 'Awaiting Compliance Approval' } 
    else 
    @concept.attributes = { :status => 'Awaiting Marketing & Legal Approval' } 
    end 
else 
    if params[:consulted_marketing] == '1' 
    @concept.attributes = { :status => 'Awaiting Marketing Approval' } 
    else 
    @concept.attributes = { :status => "Pending Approval" } 
    end 
end 

Answer 3

這看起來像是一個商業規則給我。因此，你可能需要將其包裝成一個功能：

string GetConceptStatus(bool consulted_legal, bool consulted_marketing) 
{ 
    if (consulted_legal && consulted_marketing) { 
     return "Pending Approval"; 
    } 
    if (consulted_legal && !consulted_marketing) { 
     return "Awaiting Marketing Approval"; 
    } 
    if (!consulted_legal && consulted_marketing) { 
     return "Awaiting Legal Approval"; 
    } 
    if (!consulted_legal && !consulted_marketing) { 
     return "Awaiting Marketing & Legal Approval"; 
    } 
} 

這也分開了如何bool s的在你的界面從實際執行業務規則的編碼的細節。

但是，代碼的實際結構看起來不錯，因爲它可能更好地模擬業務規則。

Answer 4

您可以使您的代碼更少依賴於這兩種條件，並使其更加靈活。

waiting_on = [] 
waiting_on << 'Compliance' unless params[:concept][:consulted_marketing] 
waiting_on << 'Legal' unless params[:concept][:consulted_legal] 
status = waiting_on.empty? ? "Awaiting #{waiting_on.join(' & ')} Approval" : 'Pending Approval' 
@concept.attributes = {:status => status} 

版本都創建和更新，無需過濾器：

def create 
    set_concept_status_attribute 
    ... 
end 

def update 
    set_concept_status_attribute 
    ... 
end 

private 
    def set_concept_status_attribute 
    waiting_on = [] 
    waiting_on << 'Compliance' unless params[:concept][:consulted_marketing] 
    waiting_on << 'Legal' unless params[:concept][:consulted_legal] 
    status = waiting_on.empty? ? "Awaiting #{waiting_on.join(' & ')} Approval" : 'Pending Approval' 
    @concept.attributes = {:status => status} 
    end 

或者用的before_filter：

before_filter :set_concept_status_attribute, :only => [:create, :update] 

def create 
    ... 
end 

def update 
    ... 
end 

如果你查看你可以移動它，它看起來甚至更好：

module ConceptHelper 
    def get_concept_status 
    ... 
    end 
end 

<%= get_concept_status %> 

Answer 5

那個廁所ks是業務邏輯，所以它應該在模型中。

你的模型可能需要幾個屬性：consulted_legal和consulted_marketing，並設置狀態的方法時，它們中的一個改變是這樣的：

before_update :set_status 

def set_status 
    if consulted_legal && consulted_marketing 
    status = "Pending Approval" 
    elif consulted_legal && !consulted_marketing 
    status = "Awaiting Marketing Approval" 
    elif !consulted_legal && consulted_marketing 
    status = "Awaiting Legal Approval" 
    elif !consulted_legal && !consulted_marketing 
    status "Awaiting Marketing & Legal Approval" 
    end 

    true # Needs to return true for the update to go through 
end 

Answer 6

你可能會認爲主管部門的名單諮詢是一個足夠固定的概念來證明名爲consulted_marketing等變量。但對於增長和乾燥（某種程度上），Id更喜歡一個部門清單。

你真正在這裏工作的是一個工作流程或狀態機器，我認爲帶有轉換的部件清單會產生最乾淨，最可增長的代碼。

代碼是數據！建模你的數據和代碼將是微不足道的。