最好的方法来干，并更好地澄清代码

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。这有帮助吗？