最好的方法來幹，並更好地澄清代碼

Question

我正在尋找更好的方式，保持我的控制器清潔和可讀。看看這個控制器的動作：

def start 
    @work_hours = params[:work_hours].to_i 

    redirect_to labor_url, :flash => {:error => I18n.t('error.invalid_post')} and return unless (1..8).include? @work_hours 
    redirect_to labor_url, :flash => {:error => I18n.t('error.working')} and return if current_user.working? 
    redirect_to labor_url, :flash => {:error => I18n.t('error.has_quest')} and return if current_user.has_tavern_quest? 

    redirect_to labor_path 
    end 

正如你所看到的，如果一個條件發生，這些做同樣的事情。他們正在設置一個flash消息並重定向到一個url（並返回）。雖然這對我來說在澄清方面看起來不錯，但我不禁注意到重定向中的一些重複，而我不喜歡以這樣醜陋的方式設置flash [：error]的翻譯。

你認爲這可以做得更好，乾燥和更可讀的方式嗎？

Answer 1

的網址是所有重定向相同的（如果我看到正確的，URL和路徑之間沒有差異），所以我會重構是如下：

def start 
    @work_hours = params[:work_hours].to_i 

    flash[:error] = I18n.t('error.invalid_post') unless (1..8).include? @work_hours 
    flash[:error] = I18n.t('error.working') if current_user.working? 
    flash[:error] = I18n.t('error.has_quest') if current_user.has_tavern_quest? 

    redirect_to labor_path 
end 

所以：如果需要設置閃光燈，和在所有情況下重定向到labor_path。這有幫助嗎？

如果在錯誤的情況下，你需要重定向到別的東西，這樣做：

def start 
    @work_hours = params[:work_hours].to_i 

    flash[:error] = I18n.t('error.invalid_post') unless (1..8).include? @work_hours 
    flash[:error] = I18n.t('error.working') if current_user.working? 
    flash[:error] = I18n.t('error.has_quest') if current_user.has_tavern_quest? 

    redirect_to labor_error_path and return if flash[:error] 
    redirect_to labor_path 
end 

如果條件不是相互排斥的，我會寫這樣的：

def start 
    @work_hours = params[:work_hours].to_i 

    flash[:error] = unless (1..8).include? @work_hours 
    I18n.t('error.invalid_post') 
    elsif current_user.working? 
    I18n.t('error.working') 
    elsif current_user.has_tavern_quest? 
    I18n.t('error.has_quest') 
    else 
    nil 
    end 

    redirect_to labor_error_path and return if flash[:error] 
    redirect_to labor_path 
end 

我不完全確定是否明確需要else nil。這有幫助嗎？