Over the last year or so, there's been lots of great tools coming out in the Ruby community that help with static analysis of code. We've now got flay, flog, reek and roodi. metric-fu runs all of these tools (plus more) to generate nice reports.
Generating nice reports is all well and good, but frankly it's not enough. We need to start enforcing code quality, not just reporting on it. The Java community is way ahead on this front, which I find rather embarrassing as a Ruby developer. Fear not though, I'm going to show you exactly how to set it up here.
Getting Started
First of all, you need a continuous integration build. Don't have one? Fail. Go have a good hard look at yourself in the mirror and consider if you really want to be a professional software developer or not.
If you're still here, you need the tools installed. That's pretty easy:
sudo gem install flay
sudo gem install flog
sudo gem install reek
sudo gem install roodi
sudo gem sources -a http://gems.github.com
sudo gem install jscruggs-metric_fu
What you've just installed is a set of tools that check for duplicate code, complex code, code smells and code problems. The last one (metric-fu) will give you a report to look at if you need to browse for more details.
Next, you need to add some tasks to your rakefile. For my rails applications, I have this in lib/tasks/quality.rake
, but you can put it anywhere that gets loaded by your build.
1 require 'flog'
2 require 'flay'
3 require 'roodi'
4 require 'roodi_task'
5 require 'metric_fu'
6
7 desc "Analyze for code complexity"
8 task :flog do
9 flog = Flog.new
10 flog.flog_files ['app']
11 threshold = 40
12
13 bad_methods = flog.totals.select do |name, score|
14 score > threshold
15 end
16 bad_methods.sort { |a,b| a[1] <=> b[1] }.each do |name, score|
17 puts "%8.1f: %s" % [score, name]
18 end
19
20 raise "#{bad_methods.size} methods have a flog complexity > #{threshold}" unless bad_methods.empty?
21 end
22
23 desc "Analyze for code duplication"
24 task :flay do
25 threshold = 25
26 flay = Flay.new({:fuzzy => false, :verbose => false, :mass => threshold})
27 flay.process(*Flay.expand_dirs_to_files(['app']))
28
29 flay.report
30
31 raise "#{flay.masses.size} chunks of code have a duplicate mass > #{threshold}" unless flay.masses.empty?
32 end
33
34 RoodiTask.new 'roodi', ['app/**/*.rb'], 'roodi.yml'
35
36 task :quality => [:flog, :flay, :roodi, 'metrics:all']
Now, add the quality
task to the list of things done in your continuous integration build. That will make sure these things run all the time. Try running rake quality
manually. It will almost certainly fail on either flog, flay or roodi. I'll show you how to tweak them one at a time.
Flog
When I ran rake flog
for the first time, I got something like this:
~/Data/runway $ rake flog
(in /Users/marty/Data/runway)
42.3: Token#parse_incubation_days
60.4: Token#parse_incubation_date
81.7: ActionParser#parse
89.7: ActionFormat#format
rake aborted!
4 methods have a flog complexity > 40
This tells me that I have four methods in my code base that have a flog complexity greater than 40. If you look back at line 11 of quality.rake
shown above, you'll see that this threshold is configurable. Set that number to something that is just higher than the most complex method in your application (in my case, I set it to 90) and run rake flog
again. It should pass.
What you've just done is create a stop loss for complexity in your application. If anyone checks in a method that is more complex than the configured threshold, your build will fail. In other words, methods are not allowed to get any more complex than the most complex method in your application now.
Unless you're working on a green fields application, I'll bet that the threshold you set was higher than you want it be. That method that was the most complex in your application needs some refactoring. You've got somewhere to focus your refactoring efforts now though. Pick that method, refactor it, and ratchet down the threshold to the next most complex method. Now you're incrementally improving the quality in your app in terms of complexity.
Flay
When I ran rake flay
for the first time, I got something like this:
~/Data/runway $ rake flay
(in /Users/marty/Data/runway)
Total score (lower is better) = 164
1) Similar code found in :defn (mass = 60)
app/controllers/signups_controller.rb:5
app/controllers/user_sessions_controller.rb:6
2) Similar code found in :if (mass = 54)
app/controllers/forgot_passwords_controller.rb:15
app/controllers/signups_controller.rb:16
3) Similar code found in :defn (mass = 50)
app/helpers/actions_helper.rb:43
app/helpers/actions_helper.rb:49
rake aborted!
3 chunks of code have a duplicate mass > 25
This tells me that I have two pieces of code with a mass greater than 30 that have been duplicated. If you look back at line 25 of quality.rake
shown above, you'll see that this threshold is configurable. Set that number to something that is just higher than the largest mass of duplicated code in your application. This can be a little confusing, because the tool reports a number equal to that mass multiplied by the number of times it appears. In my case, the reported mass of 60 is actually 30 (a mass of 30 found in 2 places). So I set the threshold to 30. Run rake flay
again. It should pass.
What you've just done is create a stop loss for duplication in your application. If anyone checks in code that duplicates an existing mass of code larger than the configured threshold, your build will fail. In other words, people can't copy and paste code in a fashion any worse than they already have.
Again, that threshold you set is almost certainly higher than you want it be. That largest mass of code that was duplicated needs some refactoring to make your code more DRY. Find that code, refactor it, and ratchet down the threshold to the next largest mass of code. Now you're incrementally improving the quality in your app in terms of duplication.
Roodi
When I ran rake roodi
for the first time, I got something like this:
~/Data/runway $ rake roodi
(in /Users/marty/Data/runway)
app/models/action_format.rb:10 - Method name "format" cyclomatic complexity is 12. It should be 10 or less.
app/models/action_parser.rb:11 - Method name "attributes" cyclomatic complexity is 12. It should be 10 or less.
app/models/action_parser.rb:30 - Method name "parse" cyclomatic complexity is 14. It should be 10 or less.
app/models/token.rb:173 - Method name "parse_incubation_date" cyclomatic complexity is 11. It should be 10 or less.
app/models/token.rb:226 - Method name "parse_incubation_days" cyclomatic complexity is 12. It should be 10 or less.
app/models/action_parser.rb:50 - Block cyclomatic complexity is 11. It should be 8 or less.
app/models/token.rb:11 - Block cyclomatic complexity is 10. It should be 8 or less.
app/models/action.rb:103 - Method "apply_defaults_from_name" has 23 lines. It should have 20 or less.
app/models/action_parser.rb:30 - Method "parse" has 23 lines. It should have 20 or less.
rake aborted!
Found 9 errors.
This tells me that I have several pieces of code failing Roodi checks. If you look back at line 34 of quality.rake
shown above, you'll see a reference to roodi.yml
. This is a config file that allows you to configure Roodi. Create that file in the root directory of your application and paste the following contents into it. It's what I have in my config file.
# AssignmentInConditionalCheck: { }
# CaseMissingElseCheck: { }
ClassLineCountCheck: { line_count: 300 }
ClassNameCheck: { pattern: !ruby/regexp /^[A-Z][a-zA-Z0-9]*$/ }
# ClassVariableCheck: { }
# TODO Get block cyclomatic complexity back down to 6 or less
CyclomaticComplexityBlockCheck: { complexity: 12 }
# TODO Get method cyclomatic complexity back down to 8 or less
CyclomaticComplexityMethodCheck: { complexity: 14 }
EmptyRescueBodyCheck: { }
ForLoopCheck: { }
# TODO Get method line count back down to 20 or less
MethodLineCountCheck: { line_count: 27 }
MethodNameCheck: { pattern: !ruby/regexp /^[_a-z<>=\[|+-\/\*`]+[_a-z0-9_<>=~@\[\]]*[=!\?]?$/ }
ModuleLineCountCheck: { line_count: 300 }
ModuleNameCheck: { pattern: !ruby/regexp /^[A-Z][a-zA-Z0-9]*$/ }
ParameterNumberCheck: { parameter_count: 5 }
This config file describes a list of checks that get run by Roodi, and some parameters that are used to initialise those checks. Have a look at http://roodi.rubyforge.org/ for details on each of those checks. Make a call on which checks are appropriate for your project, and then change the configuration on each item so that your build just passes. What you've just done is create a stop loss for these coding problems in your application. If anyone checks in code that doesn't pass these configured checks, your build will fail.
It's possible that you've had to comment out some checks that you really want on, to get the build to pass. For the ones that are still turned on, you've probably had to increase some of the thresholds. You can find that code, refactor it, and turn checks back on or ratchet down the thresholds over time. Now you're incrementally improving the quality in your app in terms of coding checks.
Summary
The installation of metric-fu will give you some nice reports to browse and find the next worst problems in your application. Run rake metrics:all
to get that report generated. Put some time aside regularly to clean up your code and ratchet down the thresholds as described above until you get them to a point you're happy with.
As far as I'm concerned, these tools are not an optional extra for your project. You must have them tools set up. It's unprofessional of you not to do everything you reasonably can to keep the quality of your code high. I hope this helps.