thinksimple.pl

Ruby static code analysis

08.05.2010 11:04

Today I’d like to show you four tools for Ruby code analysis. I’ll look at my Rails app written over two years ago, when I was just starting to learn Ruby and Rails – there is a lot to fix there.

Flog

sudo gem install flog

Shows the most complicated code you have written. The more complex the code, the higher the score, and the harder the code is to test. Here’s my result:

$ flog app/controllers/*.rb
  1045.2: flog total
    12.6: flog/method average

    68.5: EntriesController#search         app/controllers/entries_controller.rb:80
    39.6: CommentsController#destroy       app/controllers/comments_controller.rb:91
    38.6: EntriesController#search2        app/controllers/entries_controller.rb:64
    37.0: CommentsController#create        app/controllers/comments_controller.rb:49
    35.1: EntriesController#json           app/controllers/entries_controller.rb:176
    34.3: EntriesController#update         app/controllers/entries_controller.rb:47
    33.2: SessionsController#create        app/controllers/sessions_controller.rb:8
    33.2: EntriesController#advanced_search app/controllers/entries_controller.rb:115
    31.4: EntriesController#create         app/controllers/entries_controller.rb:13
    29.0: ProjectsController#update        app/controllers/projects_controller.rb:60
    27.5: EntriesController#archive        app/controllers/entries_controller.rb:137
    27.3: TagsController#update            app/controllers/tags_controller.rb:48
    26.0: ProjectsController#create        app/controllers/projects_controller.rb:43
    21.9: TagsController#show              app/controllers/tags_controller.rb:21
    21.6: TagsController#index             app/controllers/tags_controller.rb:9

EntriesController#search method is 27 lines long and includes looping over all entries and doing a regex comparison on every single one. Definitely a candidate for refactoring.

Flay

sudo gem install flay

Flay looks for structural similarities in the code, ignoring whitespace, variable names and some other changes. A sample result looks like this:

$ flay app/controllers/*.rb
Total score (lower is better) = 907                    


1) Similar code found in :defn (mass = 108)
  app/controllers/projects_controller.rb:77
  app/controllers/tags_controller.rb:66

2) IDENTICAL code found in :iter (mass*2 = 96)
  app/controllers/entries_controller.rb:16
  app/controllers/entries_controller.rb:51

3) Similar code found in :defn (mass = 90)
  app/controllers/comments_controller.rb:22
  app/controllers/projects_controller.rb:16

4) Similar code found in :iasgn (mass = 81)
  app/controllers/comments_controller.rb:9
  app/controllers/tags_controller.rb:10
  app/controllers/users_controller.rb:25

5) Similar code found in :if (mass = 56)
  app/controllers/entries_controller.rb:88
  app/controllers/entries_controller.rb:92

Flay marked a similarity in ProjectsController and EntriesController, and the similarity is the automatically generated destroy method. Not very helpful.

However, the identical code listed is assigning tags to an entry in create and update methods, and it is indeed identical. Well spotted.

Heckle

sudo gem install heckle

Heckle is a piece of really mean software. It takes your code, takes your tests, and then changes your code to see if the tests really fail. It works with Test::Unit or RSpec tests.

Unfortunately, I wasn’t able to run heckle on my current codebase, for one thing, because it doesn’t have any sensible tests (I was learning, OK?), and also because I broke my gems and now it doesn’t recognize ActiveRecord class.

Reek

sudo gem install reek

Finally, a tool for detecting code smells. Reek tells you exactly what is wrong with your code, and you better fix it. This is a sample output:

$ reek app/controllers/entries_controller.rb
app/controllers/entries_controller.rb -- 33 warnings:
  EntriesController has no descriptive comment (IrresponsibleModule)
  EntriesController#advanced_search calls entry[:content] twice (Duplication)
  EntriesController#advanced_search calls entry[:tags] twice (Duplication)
  EntriesController#advanced_search calls entry[:title] twice (Duplication)
  EntriesController#advanced_search has approx 10 statements (LongMethod)
  EntriesController#advanced_search refers to entry more than self (LowCohesion)
  EntriesController#archive calls month.nil? twice (Duplication)
  EntriesController#archive calls params 3 times (Duplication)
  EntriesController#archive has approx 11 statements (LongMethod)
  EntriesController#create calls params 4 times (Duplication)
  EntriesController#create calls params[:entry] 4 times (Duplication)
  EntriesController#create has approx 9 statements (LongMethod)
  EntriesController#create has the variable name 't' (UncommunicativeName)
  EntriesController#json calls params 6 times (Duplication)
  EntriesController#json calls params[:count] twice (Duplication)
  EntriesController#json calls params[:tag] twice (Duplication)
  EntriesController#search calls params 5 times (Duplication)
  EntriesController#search calls params[:search] 5 times (Duplication)
  EntriesController#search calls params[:search].downcase 3 times (Duplication)
  EntriesController#search contains iterators nested 2 deep (NestedIterators)
  EntriesController#search has approx 13 statements (LongMethod)
  EntriesController#search2 calls Entry.published 3 times (Duplication)
  EntriesController#search2 calls params 8 times (Duplication)
  EntriesController#search2 calls params[:search] 4 times (Duplication)
  EntriesController#search2 calls params[:tags] twice (Duplication)
  EntriesController#search2 calls params[:title] twice (Duplication)

I get it, I get it, tons of duplication and horribly long methods. Let me do some refactoring now…

(Thanks to Tomash for his presentation)

Comments

nrhm 28 May 09:31

great post, thanx.

Jason Noble 28 May 15:33

You should check out metric_fu. It runs all these for you, plus some others.

http://github.com/jscruggs/metric_fu

Comments are closed.