你会如何整理这个控制器逻辑？
我在控制器中有一些逻辑，如果满足某些条件，则设置对象的状态：
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。
这是我的看法。 我称之为Super DRY。
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所以我的代码比较字符串。 如果由于某种原因比较Fixnum s是您的情况所需要的，只需取出数字周围的引号。
您可以使您的代码更少地依赖于这两个条件，并使其更加灵活。
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}
无filter的创建和更新版本：
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 %>
这看起来像是业务逻辑，所以它应该在模型中。
您的模型可能需要几个属性：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
将其分解为嵌套的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
您可能认为所咨询的部门列表是一个足够固定的概念来certificate名为consulted_marketing等的变量。但对于增长和干燥（在某种程度上），我更喜欢一个部门列表。
你真正拥有的是工作流程或状态机，我认为带有转换的deparments列表会产生最干净，最易增长的代码。
代码就是数据！ 为您的数据建模，代码将是微不足道的。
这对我来说似乎是一个商业规则。 因此，您可能希望将其包装到函数中：
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的编码细节与业务规则的实际实现分开。
但是代码的实际结构对我来说很好，因为它可能更好地模拟业务规则。