0

このコードを実行すると、次のメソッドからできるだけ多くのifステートメントを削除する必要があります。

#  returns permissions if user is set in security context
def get_user_permissions 
  user_permissions = Set.new
  if (@user != nil)
    user_permissions << :DEFAULT_PERMISSION
    if (has_cm_team_role) 
      user_permissions << :CM_TEAM_ROLE_PERMISSION
    end
    if (has_cm_invoice_view_role || has_invoice_finance_role)
      user_permissions << :CM_INVOICE_USER_PERMISSION
      user_permissions << :INVOICE_VIEW_PERMISSION
      user_permissions << :ACCESS_ALL_INVOICE_PERMISSION
    end
    if (has_invoice_finance_role) 
      user_permissions << :FINANCE_INVOICE_PERMISSION
    end
    if (has_application_access)
      user_permissions << :CM_INVOICE_USER_PERMISSION
    end
    if (has_application_access(:CM_INVOICE_ROLE)) 
      user_permissions << :CM_ANY_INVOICE_PERMISSION
    end
    if (has_application_access(:PA_INVOICE_ROLE)) 
      user_permissions << :PA_ANY_INVOICE_PERMISSION
    end
    if (has_application_access(:SDT_INVOICE_ROLE))
      user_permissions << :SDT_ANY_INVOICE_PERMISSION
    end
  end
  user_permissions
end

私の最初の試みはほとんど機能しますが、いくつかのテストは失敗します。

def get_user_permissions 
  user_permissions = Set.new
  if (@user != nil)
    user_permissions << :DEFAULT_PERMISSION
    # add the permissions in another method
    add_permissions(user_permissions)
  end
  user_permissions
end

def add_permissions(user_permissions)
  # a hash where each key is a condition, and each value is a permission
  hash = {
    has_cm_team_role => :CM_TEAM_ROLE_PERMISSION,
    has_cm_invoice_view_role || has_invoice_finance_role => :CM_INVOICE_USER_PERMISSION,
    has_cm_invoice_view_role || has_invoice_finance_role => :INVOICE_VIEW_PERMISSION,
    has_cm_invoice_view_role || has_invoice_finance_role => :ACCESS_ALL_INVOICE_PERMISSION,
    has_invoice_finance_role => :FINANCE_INVOICE_PERMISSION,
    has_application_access => :CM_INVOICE_USER_PERMISSION,
    has_application_access(:CM_INVOICE_ROLE) => :CM_ANY_INVOICE_PERMISSION,
    has_application_access(:PA_INVOICE_ROLE) => :PA_ANY_INVOICE_PERMISSION,
    has_application_access(:SDT_INVOICE_ROLE) => :SDT_ANY_INVOICE_PERMISSION
  }

  # loop through the hash and add permissions if the key is true
  hash.each do |condition, permission|
    if (condition)
      user_permissions << permission
    end
  end

このアプローチの問題は、3つのORステートメント（ハッシュの2番目、3番目、4番目のキー）がそれぞれによって評価されないことです。だから私は次のようにProcsを使用してこれを修正します：

def add_permissions(user_permissions)
  hash = {
    Proc.new{has_cm_team_role} => :CM_TEAM_ROLE_PERMISSION,
    Proc.new{has_cm_invoice_view_role || has_invoice_finance_role} => :CM_INVOICE_USER_PERMISSION,
    Proc.new{has_cm_invoice_view_role || has_invoice_finance_role} => :INVOICE_VIEW_PERMISSION,
    Proc.new{has_cm_invoice_view_role || has_invoice_finance_role} => :ACCESS_ALL_INVOICE_PERMISSION,
    Proc.new{has_invoice_finance_role} => :FINANCE_INVOICE_PERMISSION,
    Proc.new{has_application_access} => :CM_INVOICE_USER_PERMISSION,
    Proc.new{has_application_access(:CM_INVOICE_ROLE)} => :CM_ANY_INVOICE_PERMISSION,
    Proc.new{has_application_access(:PA_INVOICE_ROLE)} => :PA_ANY_INVOICE_PERMISSION,
    Proc.new{has_application_access(:SDT_INVOICE_ROLE)} => :SDT_ANY_INVOICE_PERMISSION
  }
  hash.each do |condition, permission|
    if (condition.call)
      user_permissions << permission
    end
  end
end

OK、これは機能し、テストはすべて合格ですが、3つが正しく評価されるように、すべてのキーをProcsに変換しています。Procsは、必要な場合、つまり2番目、3番目、4番目のキーにのみ使用したいと思います。

def add_permissions(user_permissions)
  hash = {
    # just use the method name unless a Proc is required
    has_cm_team_role => :CM_TEAM_ROLE_PERMISSION,
    # use a Proc here, so that the OR is evaluated later
    Proc.new{has_cm_invoice_view_role || has_invoice_finance_role} => :CM_INVOICE_USER_PERMISSION,
    Proc.new{has_cm_invoice_view_role || has_invoice_finance_role} => :INVOICE_VIEW_PERMISSION,
    Proc.new{has_cm_invoice_view_role || has_invoice_finance_role} => :ACCESS_ALL_INVOICE_PERMISSION,
    has_invoice_finance_role => :FINANCE_INVOICE_PERMISSION,
    has_application_access => :CM_INVOICE_USER_PERMISSION,
    has_application_access(:CM_INVOICE_ROLE) => :CM_ANY_INVOICE_PERMISSION,
    has_application_access(:PA_INVOICE_ROLE) => :PA_ANY_INVOICE_PERMISSION,
    has_application_access(:SDT_INVOICE_ROLE) => :SDT_ANY_INVOICE_PERMISSION
  }

次に、キーがProcであるかどうかを判断するためのある種のメソッドで、そうである場合はそれを呼び出しますが、そうでない場合は、条件を他のキーと同じように扱います。

  hash.each do |condition, permission|
    if condition.is_proc?
      if (condition.call)
        user_permissions << permission
      end
    elsif condition
      user_permissions << permission
    end
  end
end

任意のヒント？私がこれをやろうとした方法よりも良いアイデアはありますか？これを行うことで、より周期的に複雑にしましたか？すべてのキーがProcsである作業ソリューションに固執する必要がありますか？

4

2 に答える 2

3

あなたcondition.is_a? Procの条件を満たしていますか？

于 2012-09-07T02:30:35.753 に答える
1

procの質問については何もしません。私はコードカタをやったことがありませんが、あなたが改善できると思うことはほとんどありません。

まず、条件をハッシュキーとして持つことは、私には悪い考えです。多くのキーが重複しますが、falseとtrueのみです。
逆の方法でアクセス許可をキーとして設定すると、値はtrueまたはfalseになります。

また、まったく同じ指示が3回あり、毎回2文字だけ変更されます。

if (has_application_access(:CM_INVOICE_ROLE)) 
  user_permissions << :CM_ANY_INVOICE_PERMISSION
end
if (has_application_access(:PA_INVOICE_ROLE)) 
  user_permissions << :PA_ANY_INVOICE_PERMISSION
end
if (has_application_access(:SDT_INVOICE_ROLE))
  user_permissions << :SDT_ANY_INVOICE_PERMISSION
end

それはに減らすことができます

%(cm pa sdt).each do |key|
    user_permissions << :"#{key}_ANY_INVOICE_PERMISSION" if has_application_access(:"#{key}_INVOICE_ROLE")
end

私はこれをやってみて、2つの結果を得ました。1つはハッシュを使用し、もう1つは可能な限りカットします。

# Hash version
def get_user_permissions
  return Set.new if !!@user

  user_permissions_hash = {:DEFAULT_PERMISSION => true,
                           :CM_TEAM_ROLE_PERMISSION => has_cm_team_role,
                           :CM_INVOICE_USER_PERMISSION => has_cm_invoice_view_role || has_invoice_finance_role || has_application_access,
                           :INVOICE_VIEW_PERMISSION => has_cm_invoice_view_role || has_invoice_finance_role,
                           :ACCESS_ALL_INVOICE_PERMISSION => has_cm_invoice_view_role || has_invoice_finance_role,
                           :FINANCE_INVOICE_PERMISSION => has_invoice_finance_role
  }
  %(cm pa sdt).each do |key|
    user_permissions_hash[:"#{key}_ANY_INVOICE_PERMISSION"] = has_application_access(:"#{key}_INVOICE_ROLE")
  end

  return user_permissions_hash.map {|k, v| k if v}.compact.to_set
end

# Normal version
def get_user_permissions
  return (user_permissions = Set.new) if !!@user

  user_permissions << :DEFAULT_PERMISSION
  user_permissions << :CM_TEAM_ROLE_PERMISSION if has_cm_team_role
  user_permissions << :CM_INVOICE_USER_PERMISSION if has_cm_invoice_view_role || has_invoice_finance_role || has_application_access
  if (has_cm_invoice_view_role || has_invoice_finance_role)
    user_permissions << :INVOICE_VIEW_PERMISSION
    user_permissions << :ACCESS_ALL_INVOICE_PERMISSION
  end
  user_permissions << :FINANCE_INVOICE_PERMISSION if has_invoice_finance_role
  %(cm pa sdt).each do |key|
    user_permissions << :"#{key}_ANY_INVOICE_PERMISSION" if has_application_access(:"#{key}_INVOICE_ROLE")
  end

  user_permissions
end
于 2012-09-07T02:40:52.467 に答える