10

Rubocop is always report the error:

app/controllers/account_controller.rb:5:3: C: Assignment Branch Condition size for index is too high. [30.95/24]

if params[:role]
  @users = @search.result.where(:role => params[:role])
elsif params[:q] && params[:q][:s].include?('count')
  @users = @search.result.order(params[:q][:s])
else
  @users = @search.result
end

How to fix it? Anyone has good idea?

Remi Guan
  • 21,506
  • 17
  • 64
  • 87
pangpang
  • 8,581
  • 11
  • 60
  • 96

1 Answers1

15

The ABC size [1][2] is

computed by counting the number of assignments, branches and conditions for a section of code. The counting rules in the original C++ Report article were specifically for the C, C++ and Java languages.

The previous links details what counts for A, B, and C. ABC size is a scalar magnitude, reminiscent of a triangulated relationship:

|ABC| = sqrt((A*A)+(B*B)+(C*C))

Actually, a quick google on the error shows that the first indexed page is the Rubocop docs for the method that renders that message.

Your repo or analysis tool will define a threshold amount when the warning is triggered.

Calculating, if you like self-inflicting....

Your code calcs as

(1+1+1)^2  + 
(1+1+1+1+1+1+1+1+1+1+1+1+1)^2   + 
(1+1+1+1)^2 
=> 194 

That's a 'blind' calculation with values I've made up (1s). However, you can see that the error states numbers that probably now make sense as your ABC and the threshold:

 [30.95/24]

So cop threshold is 24 and your ABC size is 30.95. This tells us that the rubocop engine assign different numbers for A, B, and C. As well, different kinds or Assignments (or B or C) could have different values, too. E.G. a 'normal' assignment x = y is perhaps scored lower than a chained assignment x = y = z = r.

tl;dr answer

At this point, you probably have a fairly clear idea of how to reduce your ABC size. If not:

  1. a simple way it to take the conditional used for your elsif and place it in a helper method.
  2. since you are assigning an @ variable, and largely calling from one as well, your code uses no encapsulation of memory. Thus, you can move both if and elsif block actions into each their own load_search_users_by_role and load_search_users_by_order methods.
New Alexandria
  • 6,951
  • 4
  • 57
  • 77
  • Thanks for you explanation – pangpang Jan 09 '16 at 04:45
  • @pangpang You should use a 'bang' (`!`) at the end of the method name, like I wrote, as this is the ruby convention for when a method mutates data / state as a result of executing. Also, thank you & glad – New Alexandria Jan 09 '16 at 05:03
  • 3
    No, that is *not* the convention. The convention is that if and *only if* there are *two* methods which do the same thing, then the one which is more surprising is named with a bang. This has *nothing* to do with mutation (see e.g. `save` vs. `save!` in ActiveRecord or `exit` vs. `exit!` in Ruby) and it *only* applies when there are *two* methods (see e.g. about a dozen methods in `Array` or `String` or `Hash` which modify the receiver, yet don't have a bang.) There should only ever be a bang method if there is also a non-bang method with the same name. – Jörg W Mittag Jan 09 '16 at 06:32
  • @jorgwmittag I see you post lots of good things. I will take you at your word. – New Alexandria Jan 09 '16 at 13:23
  • 1
    c2.com link is broken -- here's another good description: https://hub.codebeat.co/docs/software-quality-metrics#assignment-branch-condition – RubyTuesdayDONO Oct 13 '16 at 19:10