ABC score - Reduction by Combining

Previously, I discussed how to reduce an ABC score of a method by extracting out high scoring blocks of code. This time, let’s work on reducing an ABC metric score by combining high scoring parts.

This time, the code is from my version of Karger’s minimum cut. The main method which contracts a list of nodes is the min_cut:

1
2
3
4
5
6
7
8
9
10
11
def min_cut(list)
  return 0 if list.keys.count < 1

  return 0 unless connected?(list, list.keys.first, list.keys.last)
  return 1 if list.keys.count < 3 && connected?(list, list.keys.first, list.keys.last)

  node1 = list.key.first
  node2 = list[node1].first

  contract(list, node1, node2)
end

It’s a pretty straight forward block of code and I wrote it with specs. When I check with Rubocop, it gave this warning:

Assignment Branch Condition size for min_cut is too high, [21.47/15]

Wow, that’s a pretty high score for so few lines. What gives?! Time to investigate and find out how this score can be lowered.

Note: if you have about five to ten minutes, calculate the ABC score of each line in the example and compare with actual results. I bet the results will surprise you.

ABC metric score: min_cut

This is min_cut’s ABC metric scored, line-by-line:

1
2
3
4
5
6
7
8
9
10
11
def min_cut(list)                                                                      # A B C
  return 0 if list.keys.count < 1                                                      # 0 3 1

  return 0 unless connected?(list, list.keys.first, list.keys.last)                    # 0 6 1
  return 1 if list.keys.count < 3 && connected?(list, list.keys.first, list.keys.last) # 0 8 2

  node1 = list.key.first                                                               # 1 2 0
  node2 = list[node1].first                                                            # 1 1 0

  contract(list, node1, node2)                                                         # 0 1 0
end

One thing that surprised me from the last time I calculated the count: return counts as a branch.

The breakdown of Rubocop’s ABC score:

Target: High Scoring Lines

Follow similar approach as last time: focus on lowering the score on high scoring lines. In this method, the highest scoring lines are:

  return 0 unless connected?(list, list.keys.first, list.keys.last)                    # 0 6 1
  return 1 if list.keys.count < 3 && connected?(list, list.keys.first, list.keys.last) # 0 8 2

The ABC score for these lines is 14.31, which is very close to Rubocop’s ABC size default of 15. On trying to reduce the ABC score of these two lines, one thought that comes to mind: list.keys is used a lot. Four times in two lines. Each list.keys adds another branch call to the ABC score.

Reduce by Combining

Let’s reduce the number of list.keys call by combining list.keys calls into a single assignment, let’s it nodes since that is what they represent here, and use that variable in place of these calls.

This will add another assignment to the ABC metric, but it will reduce four branch calls in these two lines alone.

1
2
3
4
5
6
7
8
9
10
11
12
def min_cut(list)                                                          # A B C
  nodes = list.keys                                                        # 1 1 0
  return 0 if nodes.count < 1                                              # 0 2 1

  return 0 unless connected?(list, nodes.first, nodes.last)                # 0 4 1
  return 1 if nodes.count < 3 && connected?(list, nodes.first, nodes.last) # 0 5 2

  node1 = nodes.first                                                      # 1 1 0
  node2 = list[node1].first                                                # 1 1 0

  contract(list, node1, node2)                                             # 0 1 0
end

Overall, list.keys was used in more than those two lines, so a single assignment has reduced the number of branches by more than four, from 21 to 15:

Combine further?

Reducing an ABC score from 21 to 15 is pretty good, but can we do better?

Yes, there are still more branches than assignments and conditionals. Going back and focusing on the highest scoring lines:

  return 0 unless connected?(list, nodes.first, nodes.last)                # 0 4 1
  return 1 if nodes.count < 3 && connected?(list, nodes.first, nodes.last) # 0 5 2

By combining list.keys into nodes helped a lot. Can a similar process be applied to combine excess branch calls?

Combine: nodes.first and nodes.last

As nodes.first and nodes.last are called quite a few times in this block. Let’s combine the calls by creating new variables, first_node and last_node, and assign those values to it:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
def min_cut(list)                                                        # A B C
  nodes = list.keys                                                      # 1 1 0
  first_node = nodes.first                                               # 1 1 0
  last_node = nodes.last                                                 # 1 1 0
  return 0 if nodes.count < 1                                            # 0 2 1

  return 0 unless connected?(list, first_node, last_node)                # 0 2 1
  return 1 if nodes.count < 3 && connected?(list, first_node, last_node) # 0 3 2

  node1 = first_node                                                     # 1 0 0
  node2 = list[node1].first                                              # 1 1 0

  contract(list, node1, node2)                                           # 0 1 0
end

Similar to the first refactor, first_node was used outside of the high scoring line as well. Now the score is:

Adding first_node and last_node variables only reduced the ABC score by a few points. Most importantly, the ABC score is lower than Rubocop’s default of 15! So, there won’t be any warnings like this from Rubocop:

Assignment Branch Condition size for min_cut is too high

Reducing: when to stop?

With a score of 13.60, can we do better? Hmm… that’s a good question at this point. In a way, there will always be some way to improve, but looking at the ABC scores of each line, the highest scoring lines are still:

  return 0 unless connected?(list, first_node, last_node)                # 0 2 1
  return 1 if nodes.count < 3 && connected?(list, first_node, last_node) # 0 3 2

which is a vast improvement over the initial version:

  return 0 unless connected?(list, list.keys.first, list.keys.last)                    # 0 6 1
  return 1 if list.keys.count < 3 && connected?(list, list.keys.first, list.keys.last) # 0 8 2

Can this be reduced? Yes, it can. Can it be done simply? Not without some restructuring of the code. At this point, my first thought is to extract the block into a separate method to lower the ABC score of min_cut.

Sensible Default

Looking at the refactored version of min_cut, I find it more readable than the initial version. The ABC score default of 15 was helpful as it made my code more readable.

Rubocop’s default of 15 for ABC metric feels like a good balance of getting work done in a block but also forcing out lazy coding, which I did initially.

In this case, I just wrote the code very directly, just throwing in anything I needed as I needed. The ABC score shot up because of this.

Getting the ABC score back down to 15 really cleaned up the code without major restructuring and in a way, made the code easier to read by adding a few assignments and significantly reducing branches.

Conclusion

When Rubocop gives a warning of:

Assignment Branch Condition size for min_cut is too high. [21.47/15]

There are a few ways to deal with it.

The most important is to understand how the ABC metric is counting. Practice by counting ABC line by line.

Once high scoring lines have been identified, reduce the ABC score either by extracting blocks or by combining common elements together.