0

我正在建立评分系统，用户取决于他的正确答案，将通过或失败测试。取决于用户体验有3个标准：机构用户总是通过，专家超过3分时会通过，初学者必须有5分（100％）。为此，我创建了案例块：

def set_status(scoring_points)
  case experience_level
  when 'institutional'
    'passed'
  when 'beginner'
    scoring_points == 5 ? 'passed' : 'failed' && increase_failed_attempts
  when 'expert'
    scoring_points >= 3 ? 'passed' : 'failed' && increase_failed_attempts
  end
end

由于 CyclomaticComplexity，我想知道是否有更好、更清洁的方法来替换该案例块？也许我可以以某种方式使用警卫？

4

3 回答 3

3

要通过 CyclomaticComplexity，您可以使用一个简单的条件。

def set_status(scoring_points)
  if (experience_level == 'institutional') ||
     (experience_level == 'beginner' && scoring_points == 5) ||
     (experience_level == 'expert' && scoring_points >= 3)
    'passed'
  else
    increase_failed_attempts
    'failed'
  end
end
于 2021-04-14T16:27:08.720 回答
1

有几种方法可以重构代码，具体取决于您对scoring_pointsand的假设experience_level。

例如，如果scoring_points是一个上限为 5 的数字，那么您可以说：

PASS = {
  'beginner' => 5,
  'expert'   => 3,
}.freeze
def set_status(scoring_points)
  return 'passed' if(!PASS[experience_level] || scoring_points >= PASS[experience_level])
  increase_failed_attempts
  'failed'
end

因为scoring_points == 5和scoring_points >= 5是等价的。

您可以通过调用删除!PASS[experience_level]支票，#to_i因为nil.to_i == 0：

PASS = {
  'beginner' => 5,
  'expert'   => 3,
}.freeze
def set_status(scoring_points)
  return 'passed' if(scoring_points >= PASS[experience_level].to_i)
  increase_failed_attempts
  'failed'
end

如果您知道这experience_level将始终是您案例中的三个值之一，那么您可以添加一个显式PASS条目'institutional'而不是#to_i调用：

PASS = {
  'institutional' => 0,
  'beginner'      => 5,
  'expert'        => 3,
}.freeze
def set_status(scoring_points)
  return 'passed' if(scoring_points >= PASS[experience_level])
  increase_failed_attempts
  'failed'
end

您还可以Kernel#then或多或少地为increase_failed_attempts调用提供自己的返回值：

PASS = {
  'institutional' => 0,
  'beginner'      => 5,
  'expert'        => 3,
}.freeze
def set_status(scoring_points)
  return 'passed' if(scoring_points >= PASS[experience_level])
  increase_failed_attempts.then { 'failed' }
end

有了它，您可以更换防护装置：

PASS = {
  'institutional' => 0,
  'beginner'      => 5,
  'expert'        => 3,
}.freeze
def set_status(scoring_points)
  scoring_points >= PASS[experience_level] ? 'passed' : increase_failed_attempts.then { 'failed' }
end

我可能最终会选择第三或第四个选项。

于 2021-04-14T17:17:53.423 回答
0

我认为将其作为一种方法是错误的。问题是“机构”经验水平的处理方式与其他两个不同。具体来说，scoring_points与“机构”无关。当读者看到set_status(scoring_points)（或者应该是set_status(experience_level, scoring_points)）时，假设是需要论证。

可以写def set_status(experience_level, scoring_points = nil)和调用set_status('institutional')，但这仍然是做作的（明显做作），所以应该避免。在我看来，更好的方法是将调用替换为set_status以下之一。

#1

PASS_RANGES = { 'beginner' => 5..5, 'expert' => 3.. }

case experience_level
when 'institutional'
  'passed'
when 'beginner', 'expert'
  if PASS_RANGES.cover?(scoring_points)
    'passed'
  else
    increase_failed_attempts
    'failed'
  end
else
  # raise exception
end

如果你愿意，你可以冻结PASS_RANGES，尽管我认为没有必要这样做。

我希望increase_failed_attempts有experience_level一个论点，这让我想知道是否需要进行一些重构。

#2

case experience_level
when 'institutional'
  'passed'
when 'beginner', 'expert'
  if beginner_or_expert_pass?(experience_level, scoring_points)
    'passed'
  else
    increase_failed_attempts
    'failed'
  end
else
  # raise exception
end

def beginner_or_expert_pass?(experience_level, scoring_points)
  (experience_level == 'beginner' && scoring_points == 5) ||
  (experience_level == 'expert' && scoring_points >= 3)
end
于 2021-04-14T20:27:07.227 回答