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

Maxi Contieri
·Dec 29, 2021·

2 min read

Subscribe to my newsletter and never miss my upcoming articles

Play this article

Table of contents

  • Problems
  • Solutions
  • Context
  • Sample Code
  • Detection
  • Tags
  • Conclusion
  • Relations
  • More Info
  • Credits

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.

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 which 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] SemiAutomatic

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.

 
Share this

Impressum

Technical Opinions are my own. I don't have the revealed truth.

Software Design is a creative activity. These are hints and not rigid rules.

I write on BackEnd Business Systems and OOP Design. My advice/experience might not suit other systems.

You can write me at info(at)maximilianocontieri.com