Code Smell 112 - Testing Private Methods

Photo by Dan Nelson on Unsplash

Code Smell 112 - Testing Private Methods

If you work on unit testing, sooner or later you will face this dilemma

TL;DR: Don't test your private methods.

Problems

  • Breaking Encapsulation

  • Code Duplication

Solutions

  1. If your method is simple, you don't need to test it.

  2. If your method is complicated, you need to convert it into a Method Object.

  3. Do not make your methods public for testing.

  4. Do not use metaprogramming to avoid protection.

  5. Do not move the private computation to helpers.

  6. Do not use static methods for computations.

Refactorings

Context

We test our classes and methods.

At some point, we rely on auxiliary computations and we need to test them in a white-box way.

Sample Code

Wrong

<?

final class Star {

  private $distanceInParsecs;

  public function timeToReachLightToUs() {
    return $this->convertDistanceInParsecsToLightYears($this->distanceInParsecs);
  }

  private function convertDistanceInParsecsToLightYears($distanceInParsecs) {
      return 3.26 * $distanceInParsecs;
      // function is using an argument that is already available.
      // since it has private access to $distanceInParsecs
      // this is another smell indicator.

      // We cannot test this function since it is private
  }
}

Right

<?

final class Star {

  private $distanceInParsecs;   

  public function timeToReachLightToUs() {
    return new ParsecsToLightYearsConverter($this->distanceInParsecs);
  }
}

final class ParsecsToLightYearsConverter {
  public function convert($distanceInParsecs) {
      return 3.26 * $distanceInParsecs;
  }
}

final class ParsecsToLightYearsConverterTest extends TestCase {
  public function testConvert0ParsecsReturns0LightYears() {
    $this->assertEquals(0, (new ParsecsToLightYearsConverter)->convert(0));
  }
    // we can add lots of tests and rely on this object
    // So we don't need to test Star conversions.
    // We can yet test Star public timeToReachLightToUs()
    // This is a simplified scenario

}

Detection

[X] Semi-Automatic

This is a semantic smell.

We can only find metaprogramming abuse on some unit frameworks.

Tags

  • Test Smells

Conclusion

With this guide, we should always choose the method object solution.

Relations

More Info

Credits

Photo by Dan Nelson on Unsplash


Just as it is a good practice to make all fields private unless they need greater visibility, it is a good practice to make all fields final unless they need to be mutable.

Brian Goetz


This article is part of the CodeSmell Series.