Not everyone was a scout in his youth. Most of programmers, however, have probably heard of the Boy Scout Rule in software development. But can you name a few of the activities that are part of that rule? When I started to explore this topic, looking for what exactly needs to be done to say "I just applied the boy scout rule", I only found one thing: "Leave your code better than you found it" - everyone quotes Uncle Bob. But what does it mean? What am I supposed to do exactly?
In scouting it is easier, everyone can see if the surroundings are clean, what can be cleaned to make it cleaner, but in the code? I will try to show you a few examples from my daily work where I try to stick to this principle.
Most of us use some IDE, they are getting smarter and in many cases, they keep the code clean, suggest better solutions, "clean up" automatically with one click. However, it's worth keeping an eye on what's happening in the code and not rely entirely on the IDE.
The campground, not an entire forest!
I am allergic to multiple blank lines. When doing a code review, I also pay attention to what the code looks like. The code must be readable, multiple blank lines, in my opinion, are disturbing. So I sometimes leave a comment in CR like "one blank line is enough (everywhere)". One day my colleague got it a little bit differently than I meant. That merge request before my first review contained 7 changed files. What was my surprise when I saw it after applying the changes - 350 files changed… he removed multiple lines from the whole project. You can not do this, such changes introduce a huge disruption to the code, the MR becomes completely unreadable.
You can't go overboard with cleaning. “Leave the campground cleaner than you found it”, the campground, not an entire forest!
Code cleanup should be limited to the files we work with. If we add a new method to the class and we notice something that can be improved/cleaned up "next to" - this is the place where we should apply the Boy Scout Rule.
Examples in practice
If you use a smart IDE, it probably detects deprecated methods. The IDE I'm using strikes out deprecated names. Below, you can see crossed-out annotations @Route and @Method. Something like that is very noticeable, it's hard to miss it.
Inside the classes of these deprecated annotations, I found lines like this:
This is a very easy change to apply. They say what exactly we have to do. Just like in the screen below. A few seconds change and one thing less hurts the eyes.
It was an outdated annotation, now an example of outdated method. More crossed letters below. Method setData(). Just like a moment ago. It cannot be overlooked.
A good scout will not ignore it. It is like a plastic bottle on the ground in the campground, that needs to be cleaned up. So just check this method, maybe the author left a hint. And I did it:
Again, we have exactly what to replace.
And the last one. We're using some vendor class here that is deprecated.
Just open it:
And another quick change (but be careful with that, check if the new class has all the methods you need and they work the same way, so as not to break anything).
Of course, if you are using a less wise IDE, looking for deprecations is not that easy. Nobody in their right mind opens every class and method they use to look for information about deprecations. We just use what we know, usually without much thought. Therefore, it is worth using tools that support our work.
b) unused code
The code changes very often, especially in the early stages of development. With changes, sometimes you miss to delete an unused import, variable, or method.
A smart IDE detects unused code, for example, turns unused private methods to gray. This also works for unused imports, unused private properties and parameters.
But you have to be careful, I never trust the IDE completely, it is better to double-check that deleting a piece of code won't break something. Of course, your code is fully test covered, so you can be sure nothing's broken ;).
It's not that easy with public methods, they don't turn gray automatically ;). You need to be more careful with them. When you stop using some public method, let's just check to see if it’s used somewhere else. If not and no one needs it anymore, let's get rid of it.
We should remove unused code. That's what we use version-control systems for, so we don't have to see "maybe it will be useful someday" unused code. (YAGNI)
We have to name many things in our work. Variables, methods, classes, everything must have a name. What should a good name look like? Not too long, not too short, and clear to anyone who can see it. It is not easy at all. I believe that this is a key part of our work. A badly named variable can mess up a lot and add a lot of work to others who later have to get into our code. It should be understandable without thinking. After reading the name of the method, the names of its arguments, and what it returns, we should know exactly what it does, without looking inside.
When I see the name of the variable $a, $var, $arr or $array in the code review, I immediately get a fever.
And below is an example of a variable with the two-letter name "ac". To know what it will contain, you need to look where something is assigned, it is easy to find if it is a constructor, and not always it will be there. It is obvious to the author of this code at the time of writing that "ac" stands for "authorizationChecker". But not everyone will figure it out, even the author after a while may not be sure what "ac" meant.
A good scout, seeing such a variable, will quickly change its name to clean up a bit. For example, such a change is enough:
Don't save on letters or you'll pay for it later trying to understand what the abbreviations mean.
d) code formatting
The worst thing I have seen about improving code formatting is to use autoformat in the IDE all over the project directory ("not an entire forest"!). Let's clean around the place where we work, but not too far, one file or one method is enough. Someone who reviews the code should be able to focus on important things and not scroll through hundreds of non-essential changes.
"My formatting is the clearest and most readable" - every programmer. Everyone has their own taste and habits, so you need to agree on the details of code formatting in your team. You probably have your standards in the project you work in. However, there is one thing everyone should follow - the PSR-12 (If you missed, the PSR-2 is deprecated).
Let’s see some examples.
Just add a few spaces and new lines
or a short IF syntax
multiple blank lines
I don't know why, but very common
Just be a good scout and remove unnecessary blank lines
strange indentation in the code.
Are you uncomfortable reading it too? When you see something like this in the code, you can easily do the right thing and correct the indentation.
all other things incompatible with PSR-12
Boy Scout is not looking for whom to blame, but clean code himself, leaving the code better than he found (DO NOT apply this rule in code review, such things must be eliminated immediately! ;))
e) complex conditions
How many times have you wasted time trying to understand a complicated if condition? For me - too much. Worse, with many conditions, it is often easy to get confused (if you have tests, you are a lucky scout)
I made up an example below. let's say it's a method that handle some response. It first checks if the response is valid, if so, does something with it.
It is not overly complicated to analyze, but it does take a moment to analyze what is being checked here.
A good scout will find a few things here that he can clean up so that others will enjoy reading the code. But the most important thing is to separate the entire condition into a separate method whose name will clearly say what the condition checks.
Of course, in this example condition, many other things can be improved ;). But the point here was that we took the time to understand what it checks, so let's make it easy for other people not to waste that time.
f) code duplication
Why do we find duplicate code in projects? If something works in one place, it will work in another ;). But there is one problem. If you want to change or correct something later, you must remember to do it in all places where you copied the code fragment. Mindless copy-pasting is unacceptable and no one should do it. But we often see that it happens, all the time. When a good scout sees duplicate code, he just clean it. The smart IDE notices that the code is duplicated, it should alert you.
Here is an example to show what I'm talking about
We have two methods here, even if you search, you won't notice the difference other than the function name.
The first method sends an email with some positive decision, and the second method sends an email with some negative decision. For some reason, instead of extracting common code, someone just copied it.
Then, you have the task to change the title of sent emails, and you noticed what it looks like. Now you have two options what to do. First one is to only change the title in both places. If it works, why not? Well, NO! A good scout would do more than that. He'd clean up that piece of code.
It doesn't take much time, and the code is a bit cleaner and easier to maintain.
g) what else?
Anything that will improve the readability of the code or its maintenance. You are not doing it for others, you are doing it for yourself in the future. All shown examples above can be done in less than a minute. This is well invested time. It will pay off in the future.
Need further help?