My code reeks...

1 view
Skip to first unread message

William Hertling

unread,
Dec 2, 2009, 3:05:03 PM12/2/09
to pdxruby
I just discovered reek (http://wiki.github.com/kevinrutherford/reek),
and boy does my code stink.

I'm curious how seriously to take the output of reek. In particular, I
seem to have a lot of duplication warnings, where I refer to a
variable multiple times. Other warnings (uncommicative name, long
method, large class) seem more straight-forward to deal with. I'm not
sure what I think be thinking about to refactor out duplication
though.

Thanks,
Will

Joel Hoffman

unread,
Dec 3, 2009, 2:25:16 AM12/3/09
to pdx...@googlegroups.com
Thanks for pointing this out, it looks very useful. After a little
experimentation, it looks like up to five or ten percent of the
"smells" it reports are genuine issues. In fact, I suspect that reek
failing to complain at all is itself a code smell. But, identifying
that five or ten percent is worth a lot, especially Feature Envy,
Control Couple, Data Clump, Long Method...

....not that any of those appeared in *my* code...

joel

Erik Hollensbe

unread,
Dec 3, 2009, 5:36:32 AM12/3/09
to pdx...@googlegroups.com
If you guys enjoy these kinds of tools, 'flog' is written by Ryan Davis
and the rest of seattle.rb (ruby2ruby, parsetree, ruby_parser) and
employs a similar amount of subjective reasoning, albeit likely from a
different viewpoint... It employs a scoring mechanism (lower is better)
that indicates how easy the seattle.rb team thinks it is to test,
instead of something more like reek and perlcritic that spell out what
they think is wrong.

The other tools on this page are also in the same vein, with different
focuses. 'flay' in particular can be pretty useful to identify
low-hanging refactoring fruit.

http://ruby.sadi.st/Ruby_Sadist.html

-Erik

Igal Koshevoy

unread,
Dec 3, 2009, 7:03:25 AM12/3/09
to pdx...@googlegroups.com
I strongly recommend reading through the text description of the various
code smells:
http://wiki.github.com/kevinrutherford/reek/code-smells
I agree with Joel's assessment. Although useful, "reek" is a very whiny
and paranoid robot, and many of its concerns are overblown and not worth
resolving.

As for your concern with the Duplication smell and how to resolve it --
this smell is caused
by "repeated identical method calls within any one method definition", a
concern because this may return inconsistent results and incur a
performance hit. For example, Duplication smell would come from a method
that makes multiple calls to `Foo.count` because it's executing an
expensive query and may return different results each time it's called.
To resolve the smell, you would do something like "foo_count =
Foo.count" and use this consistent value rather than re-executing the
dynamic method.

-igal

Jason Noble

unread,
Dec 3, 2009, 8:20:12 AM12/3/09
to pdx...@googlegroups.com

You may want to take a look at the metric_fu (http://metric-fu.rubyforge.org/
) gem as well.

sudo gem install jscruggs-metric_fu

# Rakefile
require 'metric_fu'

rake metrics:all

By default, it runs Churn, Saikuro, Stats, Flog, Flay, Reek, Roody and
rcov for you. You can dynamically configure each of these (and
disable ones you don't like).

http://metric-fu.rubyforge.org/ has example graphs available.
Metric_fu also plays (moderately) well with Cruise Control.rb.

Jason
> --
>
> You received this message because you are subscribed to the Google
> Groups "pdxruby" group.
> To post to this group, send email to pdx...@googlegroups.com.
> To unsubscribe from this group, send email to pdxruby+u...@googlegroups.com
> .
> For more options, visit this group at http://groups.google.com/group/pdxruby?hl=en
> .
>
>

Erik Hollensbe

unread,
Dec 3, 2009, 8:22:51 AM12/3/09
to pdx...@googlegroups.com
On 12/3/2009 8:20 AM, Jason Noble wrote:
> You may want to take a look at the metric_fu (http://metric-fu.rubyforge.org/
> ) gem as well.
>

I'll have to check that out; we're already using rcov pretty heavily at
$work and integrated like this, it should be pretty painless to inject
into our rakefiles.

Thanks!

-Erik
Reply all
Reply to author
Forward
0 new messages