thinksimple.pl

Naming things

July 23, 2011

Am I obsessing over naming things? Is it really that much of a difference if I say loc instead of location? For a local variable? In a ~10LOC method? Yes, it occupies a part of my brain, and it’s slightly annoying once you get used to having everything pretty, but is it worth nitpicking and deny someone a few saved keystrokes?

As I’m typing this, I have already realised what the answer is. Yes. Yes it is worth it. Because the writer only writes it once, but the reader reads it multiple times. Maybe hundreds of times. And every single time this small part of the reader’s brain is occupied with decoding the variables, what the cryptic name stands for and what the variable contains. Overtime, it adds to productivity loss. Greater than saving those keystrokes. Long term, it’s not worth it. Short term… If you’re coding for short term only, then you’re either lucky, or mistaken. Probably the latter.

Ruby static code analysis

May 8, 2010

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: 2

Ruby tricks

April 25, 2009

Some time ago I posted a link to a rubyinside post about ruby tricks. As I hardly ever click through all those links I post (usually once, when I find them), I decided to paste my favourite tricks here.

First, this is a great shortcut for extracting data with regular expressions.

email = "Fred Bloggs <fred@bloggs.com>"
email.match(/<(.*?)>/)[1]            # => "fred@bloggs.com"
email[/<(.*?)>/, 1]                  # => "fred@bloggs.com"

Quick mass assignment:
I’ve seen it in Perl (ahh, remember studying those tests writing one-liners…), but good to know it’s in Ruby as well:

a, b, c, d = 1, 2, 3, 4

Wow… thought there would be more :) The rest of the tricks were either things I don’t really think are good (like point 7 – cut down on local variables definitions) or I already know of (like all the tricks with sprintf, shortened to % operator). If you’re interested, go read the article yourself :)

Ruby tricks

October 19, 2008

Something to print out and put it over your desk:
Ruby tricks

Shoes

August 22, 2008

Ruby’s cross-platform GUI toolkit