2
\$\begingroup\$

I have this method which returns the number, but Rubocop said:

Assignment Branch Condition size for rating is too high. [15.84/15]

  def rating
    response = response_quality

    if response == 1 && @typos.zero?
      5
    elsif response == 2 && @typos.zero?
      4
    elsif response == 3 && @typos.zero?
      3
    elsif @typos.percent_of(@word_length) < 20
      2
    elsif @typos.percent_of(@word_length) < 30
      1
    elsif @typos.percent_of(@word_length) > 30
      0
    end
  end

What can I change?

\$\endgroup\$
1

2 Answers 2

1
\$\begingroup\$

What happens when @typos.percent_of(@word_length) is exactly 30? You need to fill in the gaps. The last case should be an else rather than an elsif, to ensure that all cases are covered.

Is it guaranteed that response_quality will be 1, 2, or 3? If it's not obvious from context, then there should be an explanatory comment that other values are or aren't possible.

The solution below would reduce the code a bit, but it's not necessarily better than your original approach, which did have the advantage of regularity in the pattern.

def rating
  if @typos.zero?
    case response_quality
    when 1; return 5
    when 2; return 4
    when 3; return 3
    end
  end

  typo_pct = @typos.percent_of(@word_length)
  typo_pct < 20 ? 2 :
  typo_pct < 30 ? 1 :
                  0
end
\$\endgroup\$
2
  • \$\begingroup\$ response_quality always 1,2 or 3. \$\endgroup\$
    – Pavel
    Dec 11, 2015 at 0:25
  • \$\begingroup\$ But I read this and changed your ternary operator to usual if/else \$\endgroup\$
    – Pavel
    Dec 11, 2015 at 1:49
0
\$\begingroup\$

There's nothing magic about Rubocop, but it does encourage you think about what makes readable (as opposed to just correct) code. In this case, I think it is clearer to make the conditions explicit in the rating method as you've done rather than break them up into separate methods to pass Rubocop. Specifically, I would suggest this:

# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
# rubocop:disable Metrics/MethodLength, Metrics/PerceivedComplexity
def rating
  case
  when response_quality == 1 && @typos.zero?
    5
  when response_quality == 2 && @typos.zero?
    4
  when response_quality == 3 && @typos.zero?
    3
  when @typos.percent_of(@word_length) < 20
    2
  when @typos.percent_of(@word_length) < 30
    1
  when @typos.percent_of(@word_length) > 30
    0
  end
end
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity
# rubocop:enable Metrics/MethodLength, Metrics/PerceivedComplexity

Finally, you should consider whether you want rating to be nil when no conditions match, and you might consider explicitly adding an else nil to make that decision obvious.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.