Friday, May 15, 2015

Should we test private methods?

Should we test private methods?

One question which very often pops up when it comes to test-driven development is: "should we test private methods?" There are some (controversial) ways to test private methods in C#. For example, you can declare the test assembly a friend class of the testee assembly. That doesn't smell good though.

So should we test private methods?

This blog post will not try to answer that... because the question is wrong!

It's a sign

If you have read my previous blog post Help, my class is turning into an untestable monster, you may recall that untestable code is a sign of a poor design. It's time to do some refactoring.



Testing private methods is a related topic; if you feel the need to test private methods in order to test the behavior of a class, then it's a sign that you should do some refactoring. Your class is probably trying to do too much. It is violating the Single responsibility principle.

Example

Let's have a look on an example. Consider a class Logger which updates a log in a database when certain events occur. The Logger class has a method LogEvent(event) which generates a message with a timestamp based on the event and adds it to the database. It might look like this:


public class Logger
{
  public void LogEvent(AppEvent event)
  {
    string message = CreateMessage(event);
    AddMessageToDatabase(message);
  }

  private string CreateMessage(AppEvent event)
  {
    string message = "";

    foreach (var something in event.AffectedObjects()
    {
      ...do stuff here, add to string etc etc
    }

    ...add nicely formatted timestamp here

    return message;
  }

  private void AddMessageToDatabase(string message)
  {
    ...do all kinds of database magic here, add to correct table, avoid duplicates, etc
  }
}

Although a fairly trivial example, the Logger.LogEvent() method is trying to do two things: create a nicely formatted message AND add it to the database. Both of those tasks can have a number of different scenarios and failure conditions, so surely we feel the urge to test both the private CreateMessage() and AddMessageToDatabase() methods!

Refactor!

Having realized that this is a sign, we decide to refactor it:


Code:
public class MessageGenerator
{
  public string CreateMessage(AppEvent event)
  {
    ...create nicely formatted string

    return message;
  }
}

public class DatabaseLogger
{
  public void AddMessageToDatabase(string message)
  {
    ...do all kinds of database magic here, add to correct table, avoid duplicates, etc
  }
}

Now we have two smaller classes with one single responsibility each. The CreateMessage and AddMessageToDatabase methods are public because they are a part of the public contract and expressed responsibility of these classes.

Did we answer the question about whether or not to test the private methods of the Logger class? No, because the question was wrong!