When working in a mature codebase, there is a common scenario of a small change that is OK by itself, but which aggravates an existing code health problem. For example, someone may need to add another function to a file that is already thousands of lines long, or another parameter to a list of dozens, or another cut-and-paste function that is almost but not quite like several others.
Cases like this are hard because of the duality of the problem. On the one hand, the developer is only doing what many others have done before, but on the other they are definitely making things worse.
Let's begin by considering three ways of handling the situation.
1. Found a snake? Kill it.
Under this policy, whoever needs to make changes to code that has a real code health problem is responsible for making things right. They are supposed to consider the whole problem and implement a proper solution.
The real strength of this policy is its immediacy. Code is fixed as it gets touched, meaning that the most vital portions of the codebase get updated in short order.
The problem with this policy is disproportionality. A small change can turn into a huge refactoring job. And there can be second-order problems as developers twist their designs to avoid having to deal with that crawling file of horrors two directories over.
2. The Boy Scout rule.
The old rule among the Boy Scouts was to leave the campground better than you found it. In the context of coding, this means doing a little bit of cleanup when encountering an ugly bit of code, but not necessarily rewriting the whole thing. Add a test, pull common cut-and-pasted code into a function, eliminate a redundant parameter or two -- nothing too arduous.
The strengths of this policy are the continual progress it encourages and the rather modest expectations it places on developers. These modest expectations mean that the policy is actually likely to be followed.
The real weakness is slow progress -- big problems will improve only slowly. There are also some problems that are not amenable to gradual reform.
3. For everything there is a season.
Under this policy, the right thing to do when encountering a nasty bit of code is to file a bug and enter it into the owning team's list. The team then periodically (quarterly? yearly?) runs a bug bash to clean up accumulated problems.
The strength of this policy is the opportunity for prioritization before the bash. There are always more problems than there is time available for fixing them, and some are more important than others. This policy also avoids mixing changes for new features with changes to fix accumulated problems.
The weakness is the lack of immediacy; things get worse before they get better. These is also a real risk that some problems are never fixed. Some teams are very diligent about tending their bug lists; for others, the list is where bugs go to be forgotten.
A common policy
For my money, the best of these policies is the Boy Scout rule. It ensures continual progress without asking for too much, and is therefore likely to be actually followed. I also expect that the changes it calls for are typically in some of the most vital code in the codebase, since unimportant code tends to be left alone.
That said, there are definitely cases where the Boy Scout rule is inappropriate: developers who are unfamiliar with the codebase, problems that require large-scale fixes, and crisis times when there just isn't time. In such cases, it's better to file a bug for the next bug bash. But the more this is done, the more vital it becomes to actually hold those bug bashes regularly and intensively.
No comments:
Post a Comment