Our code should be clean. Yet, messes are made, left, hidden, forgotten, refound, buried, and then fed to the Ravenous Bugblatter Beast of Traal who then rains on our parade and ruins our release, wrecks our weekend—or all the above. How do we prevent this disaster?
The Boy Scouts have a well-known adage: “leave it better than you found it.” When applied to creating clean code, it could save our weekends. However, sometimes “Boy Scouting” your code can cause further complications—and disrupt your colleagues. We’ll use two examples to demonstrate when to clean and when not to.
Example 1: Terry’s clean change
Let’s talk about Terry. Terry is your stereotypical well-intended developer and is tasked to fix a simple bug. Today’s bug is a method that should be subtracting blargs from widgets, but is instead subtracting widgets from blargs.
Terry reviews the code and realizes that even though the fix is simple, there are opportunities to make improvements. Terry makes a list of the changes to plan the work:
- Add a test to duplicate the bug
- Apply the fix
- Boy Scout
- Rename variables
– ‘a’ to ‘widgets
– ‘b’ to ‘blargs’
– ‘dif’ to ‘result’
- Rename method
– ‘GetBlargs’ to ‘CalculateRemainingBlargs’
Terry commits each change separately, but when making the “Rename method” change, Terry notices that it affects a lot of unrelated areas.
Decision time. Terry could go ahead with the refactor; the method is directly related to the changes at hand and renaming the method would make 42 other areas clearer.
Yet, the bug isn’t about cleaning the code and changing those usages could obfuscate the real change. Not to mention, other team members may be working on real changes in those areas. Renaming the method risks hiding the real change and conflicts with other team members. Terry decides to create a new ticket to rename the method in a future sprint.
Terry’s pull request is clean and makes it easy to follow the change. Celebration ensues.
Example 2: Casey’s disastrous refactor
Now let’s consider Casey the budding architect. Casey is asked to pick up Terry’s ticket to rename the method responsible for subtracting blargs from widgets. The whole team is aware of the change and onboard to avoid the affected areas.
While making the change, Casey finds instances where the calling code could be improved. Casey humbly puts on a Boy Scout cap and dutifully refactors each caller.
The pull request is now hard to review and has caused conflicts with other developers’ work. Casey loses friends and the respect of others in the process, especially when they notice a bug buried in the refuse. Casey’s change is declined and his effort lost.
When you should Boy Scout
Apply the Boy Scout rule when ALL the following are true:
- Directly relates to the changes at hand
- Won’t cause conflicts with other workstreams
- There’s minimal risk of introducing bugs
If all the boxes above aren’t checked, save yourself and create a ticket.