What is the boy scout rule

The boy scouts have a rule:

Always leave the campsite better than you found it

This means they throw away the trash they see, they clean up the area, and at the very least, they don’t make it any worse.

We can apply this same rule to our code base. Always leave the code cleaner or better than you found it. Over a longer period of time, consistently applying this rule can have great impact on our code base.

But what does the boy scout rule mean in the context of our code? For a camp site its simple to see what can be done. For our code this may prove a bit more difficult. Just recently i spoke with someone who said they couldn’t apply the boy scout rule to their code, as they don’t have the time for it. When investigating a bit further, it turned out they mis understood the boy scout rule. To them it meant making every piece of code you touch perfect. And all the code that it interacts with as well. This is for many code bases close to impossible, and its also not what the boy scout rule is.

The boy scout rule is about making small, continuous improvements to your code. If you are fixing a bug, and find the code around it is a mess, fix a bit of that mess. You don’t have to make it perfect, you don’t have to fix it all, but improve it just a little bit.

Lets look at an example. Recently i found a factory class that was instantiating an object with more parameters than it needed. (Which is totally valid and not a problem for PHP) So i decided to fix that issue.

The starting code looked something like this:

<?php
class ImplementationFactory
{
  private $container;

  public function __construct(Container $container)
  {
    $this->container = $container;
  }

  /**
   * @return ImplementationInterface
   */
  public function createImplementation()
  {
    $config = $this->container->get(Config::class);
    $router = $this->container->get(Router::class);
    
    if ($config->getEnv() === 'development') {
      return new DevImplementation($config, $router);
    } else {
      return new Implementation($config, $router);
    }
  }
}

There are a few things wrong with this code. The one i was initially going to fix was the fact that these constructors no longer took arguments. But there are more:

1. The factory takes the complete container as an argument.
2. We are using an else when it is not needed.
3. We are checking the env against a string, where a constant may be more appropriate

And after all that, its also the question of whether we should have a different dev implementation for this, and whether that problem should be solved in this factory.

Lets first of all start with the task i was supposed to do:

  public function createImplementation()
  {
    $config = $this->container->get(Config::class);
-    $router = $this->container->get(Router::class);
    
    if ($config->getEnv() === 'development') {
-      return new DevImplementation($config, $router);
+      return new DevImplementation();
    } else {
-      return new Implementation($config, $router);
+      return new Implementation();
    }
  }

At this point i could close the ticket, and move on. But i apply the boy scout rule I got three points that are wrong with the code. Lets fix the first one: “The factory takes the complete container as an argument.” This isn’t necessarily wrong, but it is far more than what this factory needs to know:

class ImplementationFactory
{
-  private $container;
+  private $config

-  public function __construct(Container $container)
+  public function __construct(Config $config)
  {
-    $this->container = $container;
+    $this->config = $config;
  }

  /**
   * @return ImplementationInterface
   */
  public function createImplementation()
  {
-    $config = $this->container->get(Config::class);
    
-    if ($config->getEnv() === 'development') {
+    if ($this->config->getEnv() === 'development') {
      return new DevImplementation();
    } else {
      return new Implementation();
    }
  }
}

This is a bit better already. At this point i run the test suite, to see where i broke something. To my surprise, no tests broke. (This wasn’t that much of a surprise, as there are very little tests.)

So instead of moving on the the next point i added 2 tests. One to check that for development the DevImplementation was returned, and another one to make sure in other cases the Implementation was returned.

At this point i had spent less than 10 extra minutes on this code. I found it safe to take the 30 extra seconds to remove the redundant else statement. And while we are at it, lets replace the return annotation with a proper return type

-  /**
-   * @return ImplementationInterface
-   */
-  public function createImplementation()
+  public function createImplementation(): ImplementationInterface
  { 
    if ($this->config->getEnv() === 'development') {
      return new DevImplementation();
-    } else {
-      return new Implementation();
    }
+    return new Implementation();
  }

Now, the Config class lives in another package, so adding constants to hold the environment strings would involve adding it in that package, and updating that. Although it wouldn’t be too much extra time, this was enough for now. And whether or not this was even the correct place for this was also outside of the scope for todays work.

class ImplementationFactory
{
  private $config;

  public function __construct(Config $config)
  {
    $this->config = $config;
  }

  public function createImplementation(): ImplementationInterface
  { 
    if ($this->config->getEnv() === 'development') {
      return new DevImplementation();
    }
    return new Implementation();
  }
}

So in the span of maybe 15 minutes, i fixed what i originally set out to do, cleaned up the class a bit, and added unit tests. I know that there are still issues with this piece of code, and probably with the code that uses this class. So while this code isn’t perfect as of right now, it is better than how i found it. And that is what the boy scout rule is about.

Now, this was piece of code was relatively easy to refactor. I could see what changes could be made without too much work. Other times this may not be as simple. The key is to make improvements that can be made within a small time frame, and that are relatively easy to do.

One of the first improvements i would suggest to any piece of code is adding tests. Having tests to validate the behaviour of your code makes it easier to make changes. And even during that, remember the boy scout rule. 100% coverage may be too high of a goal for a specific class or method. But we can at least make the first step. And then the next time, we can improve that coverage a bit more.

Now if you consistently apply this rule, over time you can make big improvements. Initially the changes are small. But if a lot of small changes add up. And after while, you may even be able to make bigger changes, because the code is clean and understandable enough for it. And before you know it, the small change you apply, does make the code ‘perfect’.