0

执行此代码 kata,我必须从以下方法中删除尽可能多的 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

这种方法的问题在于每个. 所以我通过使用 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

好的,这可行,并且测试都通过了,但是我将所有键转换为 Procs,以便其中 3 个正确评估。我宁愿只在需要时使用 Procs,即第二个、第三个和第四个键,如下所示:

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问题无关。我从来没有做过代码 kata,但我认为你可以改进的地方很少。

首先,将条件作为哈希键对我来说是个坏主意。您将有许多重复的键,只有 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

我尝试自己这样做并得到了两个结果。一种使用散列,另一种则尽可能多地切割。

# 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 回答