Continuous Code Quality Improvement

While working on code in the context of a certain task, a programmer often discovers some preexisting quality issue. When this happens, there is a choice to be made:

• Fix the quality issue on the spot, and commit the fix in the context of the task at hand; or:
• Only make changes that are strictly necessary for the task at hand, and introduce a separate task for fixing the quality issue.

(Useful pre-reading: About these papers)

The practice of continuously improving the quality of the code-base is called Boy-Scouting. The term comes from a rule of the Boy Scouts of America that says: Always leave the campground cleaner than you found it. In the first chapter of Clean Code by Robert C. Martin there is a section titled The Boy Scout Rule where the author gives an adaptation of that rule for software: Always leave the code you are editing a little better than you found it.

Opinions vary as to exactly what constitutes Boy-Scouting, how it should be done, and even whether it is a good thing or a bad thing. Some people believe that Boy-Scouting should be avoided; they say that when a quality issue is discovered while working on a certain task, a new ticket must always be created and handled separately.

I once worked at a company where different development teams within the same department had different approaches: most frowned upon boy-scouting, but some embraced it. Since Boy-Scouting had a bad name overall within that company, one team that insisted on practicing it decided to rebrand it by inventing their own term for it: they called it Quality Osmosis. (TopDesk, Team Octarine 2016-2017.)

The problem

Arguments raised against Boy-Scouting include the following:

1. Boy-Scouting interferes with time tracking, because the hours it takes to complete a certain task get inflated by fixing quality issues, whereas the hours spent on fixing quality issues is unaccounted for.
   
2. Boy-Scouting renders quality improvements untraceable, because each quality improvement is hidden inside a commit for some other task.
3. Boy-Scouting adds extra burden to code review, because the reviewer has to examine not only the changes for the task at hand, but also additional changes that are unrelated to, or strictly speaking unnecessary for, or in any case not directly aiming to solve, the task at hand.

Furthermore, Boy-Scouting is viewed as undesirable in some management-heavy environments because it is seen as giving programmers too much autonomy:

• Boy-Scouting implies that programmers are free to get sidetracked, instead of being always laser-focused on (what management perceives to be) their job.
• Boy-Scouting implies that programmers have carte blanche to be making any modifications they see fit to the code base without prior authorization, so who knows what else they might be doing.

(Note that these concerns are rarely voiced, but often implied.)

Let us look at the different kinds of quality issues that may be encountered while working on a task:

• Low-impact - e.g. a misspelled identifier or a violation of formatting guidelines.
• Medium-impact - e.g. a code construct which is more convoluted than necessary and could be simplified to make it more understandable.
• High-impact: - e.g. a previously undiscovered bug.

Low-impact issues

A low impact quality issue is by definition so trivial that it is not worth creating a new task for it; imagine a ticket with the title "fix spelling mistake in identifier such-and-such"; nobody wants to go through the motions of creating, grooming, and resolving such a ticket; everyone is better off if no such ticket ever exists. So, unless the low-impact issue gets fixed the moment it is discovered, and committed as part of the task at hand, it will never get fixed. This means that we can safely establish the following rule:

When a low-impact quality issue is discovered while working on a certain task, the quality issue should be fixed on the spot, and committed as part of the task at hand.

Note that fixing a spelling mistake may involve modifying a large number of source files, which would not otherwise need to be modified in the context of the task at hand; this means that the code reviewer will have more work to do; we will address this problem later.

Medium-impact and high-impact issues

A medium-impact or high-impact quality issue does, in principle, deserve creating a separate ticket for, but first we need to ask whether it can actually be handled separately from the task at hand, because if not, then creating a separate ticket is moot point.

If it is known beyond a shadow of a doubt that the quality issue does not interfere at all with the task at hand, then the straightforward approach is in fact to create a separate ticket for the quality issue, and handle it after the task at hand has been completed; however, it is often very difficult to know for sure that a piece of code really does not affect another piece of code. There may be obscure mechanisms at play, through which a malfunction creeps through the system in non-obvious ways, and manifests in another place that nobody expected.

When we come across a code construct which is convoluted or buggy, what we essentially have in our hands is code that works in mysterious ways, so we do not necessarily know exactly how it behaves; we could try to reason about that code, throw the debugger at it if necessary, get down to the bottom of it, and determine whether it does in fact interfere or not with the task at hand, but:

• Nobody has time for that.
• It is futile, because the convoluted or buggy code has to be fixed anyway.
• Whatever conclusion you arrive to, can you actually be sure?

Ascertaining that two different parts of a software system are completely isolated from each other is difficult, in the same way that it is difficult to ascertain pretty much anything when it comes to code. (See Halting Problem.) As a matter of fact, it is so difficult, that we usually prefer to not have to ascertain things ourselves, and to write tests instead, that will ascertain things for us. However, writing tests to check whether flawed code interferes or not with other code is an exercise in futility, especially if we consider that the flawed code has to be fixed, sooner or later, anyway.

Therefore, when a quality issue is discovered while working on a certain task, and we suspect that it might be interfering with the task, it must be treated as if it does.

The bureaucracy

Now, some will insist that even if the quality issue interferes, or is suspected to interfere, with the task at hand, and must therefore be resolved before the task can be completed, the quality issue must nonetheless be resolved as a separate ticket. Let us look at the workflow necessary for that:

• Work on the task at hand must be suspended.
• A new ticket must be created for the quality issue, and given immediate priority.
  
• A new branch must be created, from master, for fixing the quality issue.
  
• A fix for the quality issue must be devised, coded, tested, and committed.
  
• The changes made in the new branch must undergo code review.
• The reviewed branch must be merged into the master branch.
• The master branch must be merged into the branch of the task at hand.
• Merge conflicts must be resolved.
• Work on the task at hand can now resume.

Note that merge conflicts are very likely to happen even if nobody else in the entire shop has touched the code in the mean time, because the quality issue was discovered while working on the task at hand, therefore the code affected by fixing the quality issue most likely overlaps with code that has already been modified in the context of the task at hand.

Also note that the programmer who is likely to fix the quality issue is the same programmer who was working on the original task, because they are probably free, since work on the original task has been suspended. When this programmer started working on the original task, they branched off from master; the code coming from master had a certain shape, and then they started making changes to it, giving it a new shape, which they are now intimately familiar with. By going back and branching off from master again in order to fix the quality issue, this programmer is now faced with the code in its original shape, which is in conflict with the shape that they have become intimately familiar with; this is brainfuck. When the programmer is done fixing the quality issue and returns to the branch of the task at hand, there is bound to be more brainfuck.

Apparently, there are no limits to the bureaucracy and the inconvenience that some people are willing to suffer in the name of some purist notion of "doing things right". I prefer a more pragmatic approach.

Enter Continuous Code Quality Improvement (CCQI)
^{_{a.k.a. Boy-Scouting / Quality Osmosis.}}

When a quality issue is discovered while working on a certain task, and the issue interferes, or it is suspected to interfere, with the task at hand, then the quality issue should be fixed on the spot, (while the programmer is in the flow,) and committed as part of the task at hand.

If someone wants to create a ticket, fine, but then a single commit will constitute a fix for multiple tickets.

Of course, an exception to this rule is the case where fixing the quality issue is going to be a month-long project in and of itself, in which case another pragmatic approach is necessary: make do with the quality issue as it is for the time being, and handle it later, when a month is available to spare.

What about time-tracking?

As mentioned earlier, Boy-Scouting interferes with time tracking, because the hours it takes to complete a certain task get inflated by fixing quality issues, whereas the hours spent on fixing quality issues is unaccounted for.

The solution to this is simple: whoever is in charge of time-tracking should be using larger error bars. The work of the programmers is difficult enough as it stands, it will not be made more difficult for the convenience of those who do time-tracking.

What about the commit history?

As mentioned earlier, Boy-Scouting renders the history of quality improvements untraceable, because each quality improvement is hidden inside a commit for some other task.

The solution to this is also simple: there shall be no traceable history for quality improvements, because quality improvements are being continuously applied to the code-base as it is being worked on.

What about code review?

As mentioned earlier, Boy-Scouting adds extra burden to code review, because the reviewer has to examine not only changes pertinent to the task at hand, but also changes that are unrelated to, or strictly speaking unnecessary for, or in any case not directly aiming to solve, the task at hand.

The solution to this is the simplest of all: the reviewer needs to get used to it.

And what about the management?

As mentioned earlier, management tends to be skeptical of Boy-Scouting because it gives programmers too much autonomy.

The solution to this is very similar to the solution for code review: the management needs to get used to it. If programmers are competent enough to complete software development tasks, they are competent enough to decide exactly what needs to be done in the context of each task.

---

Cover image by michael.gr based on "Quality" by Sutriman (CC BY 3.0), "Thumb up" by Sewon Park (CC BY 3.0), and "Five Stars" by Tyler Gobberdiel (CC BY 3.0), from the Noun Project.