Kassis Netz / blog

The latest discussion about achieving clean code lead me to write another story. This time it’s about applying the boy scout rule – but how?

Boy Scout Rule

Leave the campground cleaner than you found it.

As usual you can find a good description of this in the book Clean Code. It basically encourages you to clean up the code whenever you work on it, which in general is a pretty good thing.

However, there are times when you have to act with caution and simply sticking to the rule and change whatever comes in to your mind doesn’t help.

The Problem

At some point we decided to use new ruby hash style instead of the old hash rocket one. So far, so good, there’s nothing bad about it except that the code base at this point mostly used the old style and only some new files had been written using the new syntax.

Consider something like the following spec (outline)

context 'successful call' do
  it 'creates ...' do
    invoice
    expected_parameters = { "ser:channelId" => "312...", "ser:somethingElse" => "yadda yadda", :grossAmount => 123, :something => 'else', ... }
    ...
  end
end

Now when adding a new parameter, you could stick to the new style and add it like this

expected_parameters = { "ser:channelId" => "312...", "ser:somethingElse" => "yadda yadda", :grossAmount => 123, productCode: 12345, :something => 'else', ... }

and btw “follow the boy scout rule to make the code a bit cleaner” – at least the new attribute follows new conventions.

A Different Approach

Excerpt from Code Craft:

What happens if you’re working on code that (…) doesn’t conform to your house style? In this case it makes more sense to write code conforming to the existing style of that source file. (…) The only other real alternative is to convert the file (and any others) into your house style.

This is right and not – the boy scout rule tells us to clean up while the citation remarks to convert the complete file.

Suggestions

Here are some suggestions from my side:

• If you tend to clean up the whole file, do it in a separate commit, i.e. not mixing up with other changes. At best, don’t even put it into a separate commit in a feature branch but into a separate refactoring or cleanup branch. By that you avoid to have sudden code changes mixed with logical changes after merging into master.
• If you tend to apply boy scout rule for certain parts of a file while working on a feature, do so, but
  • put them into a separate commit into your feature branch.
  • Clean up at least for a coherent block, i. e. clean up a method, a block within a method or at least the whole line, but not just parts of a line. If you tend to clean up only one line, consider the surrounding and clean up all common lines in the current block, i. e. if your code contains consecutive lines that look somehow similar, change all of them completely.

Side Note about Ruby

This example is actually taken from real live, but to get an even cleaner code base, one would obviously apply the rules from one of the style guides, that say: Don’t mix the Ruby 1.9 hash syntax with hash rockets in the same hash literal. When you’ve got keys that are not symbols stick to the hash rockets syntax.

Following this rule would not have raised the question and this article. However I thought it’s a good example to express the weirdness of partial clean ups.