Feature Envy vs. Long Parameter List

Why is there always something wrong with code? I can’t make one improvement without sacrificing something else.

I’m not talking about trades between coupling and performance, understandability and speed. Tackle one code smell and you’ve just introduced another, a sick game of Whack-A-Mole.

Do not covet thy neighbor’s features

In the simpler, purer world of Smalltalk, I have elected to use Holub’s Builder pattern for restricted method access.

Looks innocuous enough. Don’t you just love how there are only three members in the example?

public void export( Exporter builder )
{
    builder.addName ( name.toString() );
    builder.addID ( id.toString() );
    builder.addSalary( salary.toString() );
}

Oh, this doesn’t smell at all. Now imagine this code with six members.

exportTo:exporter
    exporter addName:name.
    exporter addID:id.
    exporter addSalary:salary.
    exporter addSupervisor:boss.
    exporter addDept:dept.
    exporter addJobTitle:title.

Not so fragrant, is it? Would it be accurate to say that the Employee object is envious of the Exporter?

while(true) list.add(…)

The Fowler-approved remedy:

Fortunately the cure is obvious, the method clearly wants to be elsewhere, so you use Move Method to get it there.

All dandy.

public void exportTo(Exporter exporter)
{
    exporter.add(name, id, salary, boss, dept, title);
}

public void add(Name name, int id, Salary salary, Employee boss, Department dept, String title)
{
    addName(name);
    addID(id);
    addSalary(salary);
    addSupervisor(boss);
    addDept(dept);
    addJobTitle(title);
}

From here it is a short trip to access the Exporter’s members directly in this method.

Eh. I whacked the Feature Envy mole, and the Long Parameter List mole sprung up in its place.

Should I even bother looking up the treatment for a long parameter list?

I won’t show the example here, but needless to say, it is a typical example that doesn’t scale to real world situations. It depicts a massive parameter list of — ready? — two parameters. These two are replaced by a parameter object with a constructor with a parameter list of — ready? — two parameters.

What does that solve? It pushes the problem into another object. I don’t think the answer lies there. The question still remains: how do you reconcile Feature Envy with a Long Parameter List? Stack Overflow certainly was of no help.

I’m… Denny Crane!

Here’s the equivalent in Smalltalk.

exportTo:exporter
    exporter addName:name;
    ID:id;
    salary:salary;
    supervisor:boss;
    dept:dept;
    jobTitle:title.

Here is the new method in the Exporter.

addName:aName ID:anId salary:aSalary supervisor:boss dept:aDept jobTitle:title
    self addName:aName.
    self addID:anId.
    self addSalary:aSalary.
    self addSupervisor:boss.
    self addDept:aDept.
    self addJobTitle:title.

This calls into question just what is so objectionable about Long Parameter Lists. For me, it is entirely a matter of understandability. Smalltalk has named parameters, and I find the Smalltalk version more readable than the Java version.

Well, do I? Don’t they read kind of the same?

exportTo:exporter
    exporter addName:name;
    ID:id;
    salary:salary;
    supervisor:boss;
    dept:dept;
    jobTitle:title.

sounds like “Dear Exporter, add name, id, salary, boss, dept, and title to yourself.”

exporter.add(name, id, salary, boss, dept, title);

also sounds like “Dear Exporter, add name, id, salary, boss, dept, and title to yourself.”

Nah. This is but a special case. Most long parameter lists I’ve seen in the wild have sent me running for the nearest ledge.

So, does Smalltalk even suffer from this problem? Simulating named parameters in a language that does not support them devolves into Feature Envy.

I turn on my radio in the middle of the night…

Perhaps this is an issue of what really is Feature Envy and what isn’t. I have a feeling that the original motivation for this smell is blatant violations of Tell Don’t Ask:

We’ve lost count of the times we’ve seen a method that invokes half-a-dozen getting methods on another object to calculate some value.

Let me share something about Tell Don’t Ask that I’ve realized: the most harmful instances that require attention are those where you get some values out of an object, do something with them, and set some others back into the object.

This isn’t the case with the Exporter, even though it technically smells. Employee is calling setters on the Exporter to configure it for use.

Might this be why there are no popular static analysis tools that will tell me if my code stinks?

…and I heard things I need to know

Feature Envy is about coupling. Even Holub admits that getters and setters of some kind are needed in every OO system that has a procedural boundary layer, and unless you are writing unrealistic example code for textbooks, this is always the case.

Long Parameter Lists are also about coupling, to a lesser extent. You are coupled to the order of the parameters.

If they’re both about coupling, then what exactly are we running from?

Irreconcilable differences

Unless you plan on writing God objects, at some point, some part of your code will be somewhat coupled to the internals of your object. Just try to lessen the coupling as much as you can.

And by “you”, I mean me.

Announcer: You’re reading the EIP web-ring.
About these ads

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Follow

Get every new post delivered to your Inbox.

%d bloggers like this: