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ではなく、Fixnumなので、私のコードは文字列を比較します。何らかの理由でFixnumsを比較することが状況に必要な場合は、数字の周りの引用符を取り出してください。

于 2009-01-23T12:48:46.943 に答える
3

これはビジネス ロジックのように見えるので、実際にはモデルに含まれているはずです。

モデルにはおそらく、consulted_legal と Consulted_marketing の 2 つの属性と、これらのいずれかが次のように変更されたときにステータスを設定するメソッドが必要です。

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 に答える
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 に答える