2

如果满足某些条件，我在控制器中有一些逻辑可以设置对象的状态：

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。

4

6 回答 6

5

您可以减少代码对这两种条件的依赖，并使其更加灵活。

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 %>
于 2009-01-23T12:54:49.897 回答
3

这是我的看法。我称之为超级干燥。

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哈希中的值总是 Stringss ，而不是Fixnums 所以我的代码比较字符串。如果出于某种原因比较Fixnums 是您的情况所需要的，只需去掉数字周围的引号。

于 2009-01-23T12:48:46.943 回答
3

这看起来是业务逻辑，所以它真的应该在模型中。

您的模型可能需要几个属性：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
于 2009-01-23T13:50:13.873 回答
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
于 2009-01-23T12:49:06.007 回答
1

您可能认为咨询的部门列表是一个足够固定的概念，可以证明名为 Consulted_marketing 等变量的合理性。但是对于增长和干燥（在某种程度上），我更喜欢部门列表。

您在这里真正拥有的是工作流或状态机，我认为带有转换的部门列表将产生最干净、最可增长的代码。

代码就是数据！为您的数据建模，代码将很简单。

于 2009-03-17T16:45:37.667 回答
0

对我来说，这看起来像是一条商业规则。因此，您可能希望将其包装成一个函数：

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 如何在接口中编码的细节与业务规则的实际实现分开。

但是代码的实际结构对我来说看起来不错，因为它可能更好地模拟了业务规则。

于 2009-01-23T12:51:34.567 回答