Release 1.1.3.16

6 views
Skip to first unread message

Kevin Rutherford

unread,
Sep 19, 2009, 5:02:01 PM9/19/09
to ruby...@googlegroups.com
Hi,
Reek v1.1.3.16 is now up on github, with a single change:

The UtilityFunction checker always ignored method that made no calls
to other objects. But sometimes this can disallow a simple "helper"
method such as

def lookup(token)
TOKENS[token]
end

So now the default behaviour is that a method can only invoke the
UtilityFunction smell if it makes at least two method calls elsewhere;
and the value "2" is configurable. When I get around to imlementing
profiles (http://github.com/kevinrutherford/reek/issues/#issue/47)
it's likely that this value will drop back to "1" in the most severe
ninja profile.
Cheers,
Kevin
--
http://www.kevinrutherford.co.uk -- agile, TDD, XP, lean, TOC

Ashley Moran

unread,
Sep 19, 2009, 6:51:53 PM9/19/09
to ruby...@googlegroups.com

On 19 Sep 2009, at 22:02, Kevin Rutherford wrote:

> The UtilityFunction checker always ignored method that made no calls
> to other objects. But sometimes this can disallow a simple "helper"
> method such as
>
> def lookup(token)
> TOKENS[token]
> end


Hi Kevin

I'm puzzled by this one - why not use a class method?

Ta
Ashley

--
http://www.patchspace.co.uk/
http://www.linkedin.com/in/ashleymoran
http://aviewfromafar.net/

Kevin Rutherford

unread,
Sep 20, 2009, 4:15:07 AM9/20/09
to ruby...@googlegroups.com
>> def lookup(token)
>>  TOKENS[token]
>> end
>
> I'm puzzled by this one - why not use a class method?

As it stands, the call looks like

lookup '$fred'

Which isn't as succinct or readable as

MyClass.lookup '$fred'

I suppose

@tokens['$fred']

is nearly ok though...

In general I'm having huge conceptual difficulty with UtilityFunction;
probably because Reek hasn't yet strayed into much analysis of
classes-as-instances-of-Module. Yet...

Ashley Moran

unread,
Sep 21, 2009, 4:24:59 PM9/21/09
to ruby...@googlegroups.com

On 20 Sep 2009, at 09:15, Kevin Rutherford wrote:

> In general I'm having huge conceptual difficulty with UtilityFunction;
> probably because Reek hasn't yet strayed into much analysis of
> classes-as-instances-of-Module. Yet...

Do you mean by this that class methods should be considered utility
functions unless they don't call methods (or do something strange,
like access an instance variable) on that class object?

Kevin Rutherford

unread,
Sep 21, 2009, 4:48:52 PM9/21/09
to ruby...@googlegroups.com
Hi,

On Mon, Sep 21, 2009 at 9:24 PM, Ashley Moran
<ashley...@patchspace.co.uk> wrote:
>
> On 20 Sep 2009, at 09:15, Kevin Rutherford wrote:
>
>> In general I'm having huge conceptual difficulty with UtilityFunction;
>> probably because Reek hasn't yet strayed into much analysis of
>> classes-as-instances-of-Module. Yet...
>
> Do you mean by this that class methods should be considered utility
> functions unless they don't call methods (or do something strange,
> like access an instance variable) on that class object?

I'm kinda moving in that direction. Because each class is (represented
by) and instance of Class. But that may be a red herring. Part of me
thinks that a class is a class, ie. a language construct; and whatever
represents it at runtime is nice but irrelevant.

But to be honest, I need to sit down with some diagrams and at least
one other person to think it through.
Any takers?

Ashley Moran

unread,
Sep 21, 2009, 5:10:57 PM9/21/09
to ruby...@googlegroups.com

On 21 Sep 2009, at 21:48, Kevin Rutherford wrote:

> I'm kinda moving in that direction. Because each class is (represented
> by) and instance of Class. But that may be a red herring. Part of me
> thinks that a class is a class, ie. a language construct; and whatever
> represents it at runtime is nice but irrelevant.

I'm not sure I agree - to say that would be to undermine Ruby's
everything-is-an-object principle. It just so happens (as far as I'm
concerned) that objects are wired up to look for message responses in
their class. Maybe classes are the refactoring of the Long
MethodMissing code smell :)


> But to be honest, I need to sit down with some diagrams and at least
> one other person to think it through.
> Any takers?

I'm happy to help out. In fact it'd benefit me too - some of the
(mainly .Net plus some Java) guys at Agile Yorkshire want me to do a
brief intro to developing in Ruby. Lemme know if I'd be useful.

Ash

Kevin Rutherford

unread,
Sep 22, 2009, 4:14:56 AM9/22/09
to ruby...@googlegroups.com
Hi Ash,

>> Part of me
>> thinks that a class is a class, ie. a language construct; and whatever
>> represents it at runtime is nice but irrelevant.
>
> I'm not sure I agree - to say that would be to undermine Ruby's
> everything-is-an-object principle.  It just so happens (as far as I'm
> concerned) that objects are wired up to look for message responses in
> their class.  Maybe classes are the refactoring of the Long
> MethodMissing code smell :)

There are always two views of any code, imo: specification-time and
run-time. Code smells are, I think, (very nearly) 100% in the
specification-time space; they deal with the code as written: does
this name communicate well, is this code fragment coherent with those
around it, will these two fragments change at different times, and so
on; aspects of the specificaiton that don't affect behaviour. Classes
are the means by which we describe the behaviour of runtime objects.
It's nice and sexy that we also have runtime objects representing
those classes, but I don't necessarily see them as being the same
beasts. One is a specification, the other is a runtime mechanism.

So one view of FeatureEnvy and UtilityFunction is that they help to
improve the cohesiveness of the specification, by encouraging us to
move code fragments closer to the object state on which they operate.
And this is where it begins to get hazy for me...

Consider a "helper" method that is only called by the methods of a
single class, but which makes no use of the state of that class's
instances; where should its specification live? Shoulld it live with
the class that uses it, or should it move towards the data on which it
operates? Presumably it exists in order to help simplify the caller's
interaction with something -- I'll posit that such methods are usually
Adapters. So which is worse -- have the adapter live with the adaptee
or with the only client? Should it only move to the adaptee, or indeed
into a new class, when there's a need for a second client, perhaps?

One might argue that moving it to the adaptee creates a case of
SpeculativeGeneralisation. In which case, is UtilityFunction a smell
at all? Or FeatureEnvy, for that matter? In which case, perhaps Flay
is the best tool for dealing with those smells...?

And on the other hand, moving the fragment towards the adaptee can
help to encapsulate it better, often allowing it to expose less to the
outside world. But is thhat necessary when there's no other
duplication around...?

I'm in way too deep here,

Ashley Moran

unread,
Sep 24, 2009, 2:35:54 PM9/24/09
to ruby...@googlegroups.com

On 22 Sep 2009, at 09:14, Kevin Rutherford wrote:

> Consider a "helper" method that is only called by the methods of a
> single class, but which makes no use of the state of that class's
> instances; where should its specification live? Shoulld it live with
> the class that uses it, or should it move towards the data on which it
> operates? Presumably it exists in order to help simplify the caller's
> interaction with something -- I'll posit that such methods are usually
> Adapters. So which is worse -- have the adapter live with the adaptee
> or with the only client? Should it only move to the adaptee, or indeed
> into a new class, when there's a need for a second client, perhaps?

Hmmm, you make it sound like Utility Function. often accompany
Primitive Obsession/ Open Secret. Is that the case. If so, would you
consider creating new domain models the same as creating an adapter
method? And if so, does this solve the problem without a conflict?
(That line of thinking may be faulty from the first statement, though.)

Why wait for two clients before making a domain model? Isn't the
enhanced communication worth the refactoring?

> One might argue that moving it to the adaptee creates a case of
> SpeculativeGeneralisation.

Given that the code is actually the same, is moving it to a more re-
usable place really generalisation?


> In which case, is UtilityFunction a smell
> at all?

If you didn't count it as a code smell, what would stop me writing
every class as a set of methods that pass struct-like data around in
parameters? Maybe there is a subclass of Utility Function that is
acceptable?

> Or FeatureEnvy, for that matter? In which case, perhaps Flay
> is the best tool for dealing with those smells...?

Well this would go with passing structs around - if you allow Utility
Function you'd surely have to allow Feature Envy too.

(Personally, Feature Envy is the code smell I use most often to help
me guide the way I structure of my code. Don't know if that's good,
but it's how I work currently.)


> And on the other hand, moving the fragment towards the adaptee can
> help to encapsulate it better, often allowing it to expose less to the
> outside world. But is thhat necessary when there's no other
> duplication around...?

It sounds like you're approaching this with a mindset of:

* this (Utility Function) is common code smell, more common than
Speculative Generalisation
* refactoring it takes effort
* there is no obvious gain
* effort should only be applied to improve code when there is an
obvious gain
* we should leave the Utility Functions

But imagine a world where people did more Speculative Generalisation,
then:

* this (Speculative Generalisation) is common code smell, more common
than Utility Function
* refactoring it takes effort
* there is no obvious gain
* effort should only be applied to improve code when there is an
obvious gain
* we should leave the Speculative Generalisation

If you approach them from an neutral point, does this transform the
problem from one code smell vs another to one code structure vs
another? (And which structure do you prefer, anyway?)


Not sure how much that helps, it's about the limit of my understanding
of the problem...


Ashley

Kevin Rutherford

unread,
Sep 24, 2009, 3:34:53 PM9/24/09
to ruby...@googlegroups.com
Hi Ash,

>> In which case, is UtilityFunction a smell
>> at all?
>
> If you didn't count it as a code smell, what would stop me writing
> every class as a set of methods that pass struct-like data around in
> parameters?

My other assertions, namely that Getters/Setters and Duplication
should be completely eliminated.

> (Personally, Feature Envy is the code smell I use most often to help
> me guide the way I structure of my code.  Don't know if that's good,
> but it's how I work currently.)

All I'm suggesting -- without a shred of supporting evidence* -- is
that leaving a UtilityFunction / FeatureEnvy unmedicated may be fine
/if it isn't duplicated/. One code fragment living nearer to the
caller than the callee communicates quite well as is; but introduce
another copy and that's the time to push it towards the common callee.

Furthermore, a very large percentage of the FeatureEnvy code I've seen
makes use of getters/setters; IMO they are a worse smell, and fixing
them will also (tend to) fix the FeatureEnvy.

So I'm wondering whether there is any point worrying about
UtilityFunction / FeatureEnvy in a tool such as Reek. Maybe, just
maybe, it's a red herring to ask the user to fix that smell first. In
code with no getters/setters and no duplication, I posit that any
remaining FeatureEnvy will be harmless.
Cheers,
Kevin

* I have just remembered one thing that has nagged me for a while:
Often when I've had to fix FeatureEnvy or UtilityFunction in Reek's
own code (because Reek itself reported it), I've not been entirely
convinced that the result was always an improvement. Sometimes it
feels as if the original was more natural, less contrived. And
sometimes FeatureEnvy is /desirable/ when the alternative would be to
push together two sets of code that will change at different rates.

Ashley Moran

unread,
Sep 26, 2009, 10:57:04 AM9/26/09
to ruby...@googlegroups.com

On 24 Sep 2009, at 20:34, Kevin Rutherford wrote:

> Furthermore, a very large percentage of the FeatureEnvy code I've seen
> makes use of getters/setters; IMO they are a worse smell, and fixing
> them will also (tend to) fix the FeatureEnvy.
>
> So I'm wondering whether there is any point worrying about
> UtilityFunction / FeatureEnvy in a tool such as Reek. Maybe, just
> maybe, it's a red herring to ask the user to fix that smell first. In
> code with no getters/setters and no duplication, I posit that any
> remaining FeatureEnvy will be harmless.


So, is the fundamental point of reek to fix design issues that
encourage accidental duplication? Or are there other key purposes
with comparable benefits?

Kevin Rutherford

unread,
Sep 26, 2009, 11:11:05 AM9/26/09
to ruby...@googlegroups.com
Hi Ash,

> So, is the fundamental point of reek to fix design issues that
> encourage accidental duplication?  Or are there other key purposes
> with comparable benefits?

I don't know :)
I plan soon to spend a month with a version of Reek that's configured
to report everything except Feature Envy / Utility Function, and with
an added check for attr_reader & co. I suspect it may be quite a
traumatic period, and I have no idea whether I'll want to revert back
afterwards...
Cheers,
Kevin

Ashley Moran

unread,
Sep 26, 2009, 11:31:26 AM9/26/09
to ruby...@googlegroups.com

On 26 Sep 2009, at 16:11, Kevin Rutherford wrote:

> I don't know :)
> I plan soon to spend a month with a version of Reek that's configured
> to report everything except Feature Envy / Utility Function, and with
> an added check for attr_reader & co. I suspect it may be quite a
> traumatic period, and I have no idea whether I'll want to revert back
> afterwards...

Make sure you post the results of your experiment, if you live to
speak of it :) As someone who relies on feature envy more than any
other detector, I'd be scared of living without it...

Reply all
Reply to author
Forward
0 new messages