Code Smell 112 - Testing Private Methods
Tags: oop, testing, webdev, tutorial
Published: 2021-12-29 15:19:41

*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](https://dev.to/mcsee/laziness-i-meta-programming-32a9) to avoid protection.

5. Do not move the private computation to [helpers](https://dev.to/mcsee/code-smell-22-helpers-2j6j).

6. Do not use [static methods](https://dev.to/mcsee/code-smell-18-static-functions-fpj) 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

[Gist Url]: # (https://gist.github.com/mcsee/47c21b9a717ee1ac9b7d2f10bbb7a2d0)
```php
<?

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

[Gist Url]: # (https://gist.github.com/mcsee/acf7c184cef3babb5251da6ea946f5ae)
```php
<?

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

{% post https://dev.to/mcsee/code-smell-21-anonymous-functions-abusers-47f5 %}

{% post https://dev.to/mcsee/code-smell-22-helpers-2j6j %}

{% post https://dev.to/mcsee/code-smell-18-static-functions-fpj %}

# More Info

- [Testing Private Methods Guide](http://shoulditestprivatemethods.com/)

- {% post https://dev.to/mcsee/laziness-i-meta-programming-32a9 %}

# Credits

Photo by <a href="https://unsplash.com/@danny144">Dan Nelson</a> on <a href="https://unsplash.com/s/photos/private">Unsplash</a>
  
* * *

> 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_
 
{% post https://dev.to/mcsee/software-engineering-great-quotes-26ci %}

* * *

This article is part of the CodeSmell Series.

{% post https://dev.to/mcsee/how-to-find-the-stinky-parts-of-your-code-1dbc %}