Sunday, January 4, 2015

Unit testing smells: The method is not public

A recurring pattern emerge often when unit testing, you find an interesting method you want to test but that method is private. Of course you could call that method through other public methods on the class but it might not be easy to cover all cases. This usually is a design smell for a Single Responsibility Principle violation. More on this later.

For now, to properly test the private method we have the following options:
  • Use reflection
  • Change the method visibility to public or internal
  • Extract the method to another class

Again, if you previously read my other post on non public class and inaccessible constructor you know you should avoid using reflection in tests.

Let's take a look at the other options

Change the method visibility to public or internal

The first thing we should do is to challenge why that method is private in the first place. Why can't we just change its visibility to public or internal (and use the InternalsVisibleTo attribute)?

This breaks encapsulation of the class and will let developers call the method directly in production code. That might not be what you intended to do but can help you in the short term. Abusing this technique will burn you in the long run so be careful with it.

Fortunately, there is a simpler way: extract the method to another class.

Extract the method to another class

If you are lucky, the method you want to extract is static and don't call any other method on the original class. You should be able to extract that method to a static utility class.

But what to do when the method calls other methods and use fields or properties on the class? For that we must analyze the class to extract responsibilities using the Single Responsibility Principle.

Single Responsibility Principle

Can you tell what is the purpose of your class in one sentence without using words like: and, but, also, or, etc.? If not then your class is doing more than one thing. Each segments of the sentence could be in different classes that only have one responsibility each. If the method you want to extract use the same fields than other methods you may want to push them by parameter instead before extraction to the other class.

Find more about the Single Responsibility Principle in an old post of mine.

No comments: