Analyzability

sandal edited this page Jan 3, 2012 · 14 revisions

When code structure gets complex, it becomes harder to analyze. Difficult to analyze code makes bug fixes and improvements more challenging, because it is hard to modify code that you don't fully understand.

One way to identify complex Ruby code is via the Flog gem. It uses a fairly simple but effective metric which measures assignments, branches, and calls (an ABC metric) and computes a weighted score for each method in your codebase. This is somewhat similar to measuring cyclomatic complexity but with Ruby specific weightings.

As an example, we can take a look at the stack_wars code that I wrote for Practicing Ruby 2.10.

Here's what the report from flog looked like the first time I ran it:

seacreature:stackwars$ flog -g lib
   350.4: flog total
     8.3: flog/method average

   217.7: StackWars total
    50.3: StackWars::Game#game_over        lib/stack_wars/game.rb:37
    27.9: StackWars::Battlefield#adjacent? lib/stack_wars/battlefield.rb:42
    27.4: StackWars::Game#play             lib/stack_wars/game.rb:15
    23.5: StackWars::TextDisplay#none
    20.8: StackWars::Battlefield::from_json lib/stack_wars/battlefield.rb:7
    18.7: StackWars::TextDisplay#to_s      lib/stack_wars/text_display.rb:17
    16.1: StackWars::TextClient#play       lib/stack_wars/text_client.rb:7
    11.4: StackWars::Battlefield#deployed_armies lib/stack_wars/battlefield.rb:47
    11.0: StackWars::TextClient#point      lib/stack_wars/text_client.rb:21
    10.7: StackWars::Territory#baseline_for? lib/stack_wars/territory.rb:56

The highest scoring method was Game#game_over, but at a glance this looked like it wouldn't be trivial to refactor. In search of low hanging fruit, I went on to take a look at Battlefield#adjacent? and found the following code:

def adjacent?(pos1, pos2)
  (pos1.row == pos2.row && (pos1.column - pos2.column).abs == 1) ||
  (pos2.column == pos2.column && (pos1.row - pos2.row).abs == 1)
end

While this code isn't too hard to read, it does have a fair bit of complexity to it. It fires off about 20 method calls in just two lines of code, and has a nested logical structure equivalent to (a && b) || (c && d) which creates a surprisingly large number of branches. With some refactoring, we can trade a bit of terseness for clearer, simpler code:

def adjacent?(pos1, pos2)
  horizontally_adjacent?(pos1, pos2) || vertically_adjacent?(pos1, pos2)
end

def horizontally_adjacent?(pos1, pos2)
  pos1.row == pos2.row && (pos1.column - pos2.column).abs == 1
end

def vertically_adjacent?(pos1, pos2)
  pos1.column == pos2.column && (pos1.row - pos2.row).abs == 1
end

This refactoring distributes the branching and calls across several methods. While the flow is still ultimately similar to the original code, each individual part is much easier to understand and reason about. Running Flog again removed adjacent? from its top ten list of offenders, and showed that the horizontally_adjacent? and vertically_adjacent? methods scored a little less than half the complexity of the original compound statement:

$ flog -g lib
   351.4: flog total
     8.0: flog/method average

   215.9: StackWars total
    50.3: StackWars::Game#game_over        lib/stack_wars/game.rb:37
    27.4: StackWars::Game#play             lib/stack_wars/game.rb:15
    23.5: StackWars::TextDisplay#none
    20.8: StackWars::Battlefield::from_json lib/stack_wars/battlefield.rb:7
    18.7: StackWars::TextDisplay#to_s      lib/stack_wars/text_display.rb:17
    16.1: StackWars::TextClient#play       lib/stack_wars/text_client.rb:7
    13.0: StackWars::Battlefield#horizontally_adjacent? lib/stack_wars/battlefield.rb:46
    13.0: StackWars::Battlefield#vertically_adjacent? lib/stack_wars/battlefield.rb:50
    11.4: StackWars::Battlefield#deployed_armies lib/stack_wars/battlefield.rb:54
    11.0: StackWars::TextClient#point      lib/stack_wars/text_client.rb:21
    10.7: StackWars::Territory#baseline_for? lib/stack_wars/territory.rb:56

Because this was only a simple extract method refactoring on a two line function, it might feel a bit contrived. However, if you consider the difference between adding a bit of new functionality such as diagonal adjacency to the first implementation vs. this revised one, it should be pretty clear what the benefit is here.


Turn the page if you're taking the linear tour, or feel free to jump around via the sidebar.

You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.
Press h to open a hovercard with more details.