Extreme Enthusiasm

This is another thing I often see people do. Consider this code fragment:

<div id="sidebar">
  <%= render partial: "sidebar" %>
</div>

This basically says that in a portion of a layout that is called “sidebar” we want to render a partial template that is also called “sidebar”. Hmmmm. Duplication here! Why are these two things called the same? Let’s look into the “sidebar” template; we discover that it contains code that produces a navigation menu. Aha! Why not call the partial “navigation_menu” then?

The mistake in the above code was that the partial template was named after where it is used, not after what it is. When you correct that problem, what you get is

<div id="sidebar">
  <%= render partial: "navigation_menu" %>
</div>

and the duplication problem also disappears.

Another example I’ve seen in production code:

class Money {
  public int getValueForAuthorizationSystem() {
    return value.scaleByPowerOfTen(2).intValue();
  }   
}

Class Money should not really care what the Authorization System wants… it should be concerned with returning the amount in cents!

class Money {
  public int inCents() {
    return value.scaleByPowerOfTen(2).intValue();
  }   
}

Why is it that programmers name things after where they are used and not after what the things really are? I think that one major cause is the desire to break up a monolithic thing, because this is what design should be about, but doing it poorly. Suppose I have a 100 lines long method.

def long_method
  # ... ... ...
end

I use extract method a number of times and I end up with something like this:

def long_method
  # ...
  step_one(foo, bar, baz)
  step_two(foo, bar, zork)
  step_three(agh, foo, bar, zork)
  step_four(agh, bar, baz, zork)
end

what happened here? We took a procedure and cut it in arbitrary pieces. The result is that we have a number of methods that we can’t give meaningful names to, other than “step_X” and that have an excessive number of arguments. This means that we cut the procedure poorly. Note that each of the methods can only be used in the place where it is called; it depends on the context and can’t be used anywhere else.

What to do then? What I would do is to get back to the 100-lines method and try to cut it another way. Perhaps we will arrive at something like

def formerly_long_method
  do_something(foo)
  do_something(bar)
  do_something(baz, do_something(zork))
end

where the same helper method has been used in more than one way.

In conclusion: names like foobarForX should always be challenged. They might be fixed by looking again at what “foobarForX” really is; or they might be an indication that the design is cut the wrong way. In that case you try the cut again and see if you can improve the situation. Look for things that are meaningful concepts in the problem domain!

This entry was posted on 9 July 2012 at 14:51 and is filed under Agile. You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.