Monday, June 9, 2014

Why You Want to Test Private Methods

As programmers, we want to leave behind an API that "can't go wrong," that doesn't cause bugs by being used in the wrong order. Often the first tool we grab to do this is private methods.

This leads to a classic question.
How do I test private methods?
And the standard answer, why do you want to?

It's a response that makes one feel like a wise, bearded, mysterious master of code. But I already know the answer, for the vast majority of cases. The answer to why someone wants to test a private method is:

They shouldn't be using a private method.

At least, this is the answer 99% of the time.

But Not Me, Seriously

Odds are against you. Lets see what private methods should not necessarily be used for. Odds are pretty high that your reason for using private methods is in this list.
  • Keeping inner data structures coherent
  • Reducing duplicate code
  • Isolating pure functions from stateful ones
But I've made a list of cryptic nonsense. This is a time for examples.

Keeping inner data structures coherent
class Teacher {
    private String[] students;

    // Use this instead of adding directly to student, so
    // that its always alphabetical.
    private void addStudent(String student) {
        students.add(student);
        alphabetizeStudents(); 
    }
    // some public methods... 
}

Here we wrote a private method to handle writes into our array of students, and maintain their coherence - in this case, alphabetization. You then think, alphabetization is easy to test. How do I test this?

Reducing duplicate code
class StudentCsv {
    private writeLine(String... vals) {
       bool first = true; 
        for(String val : vals) {
            File.write(val);
            if(!first) File.write(',');
            else first = false;
       }
    }
    // some public methods... 
}

Here we found yourself writing a lot of comma management in this CSV generator. You think, lets abstract this into a method. And comma separation is easy to test, but now its private. How do I test this?

Isolating pure functions from stateful ones
class CollisionDetector {
    public long euclidianDistance(Point a, Point b) {
       return sqrt(sqr(a.x - b.x) - sqr(a.y - b.y));
    }
    // some public methods... 
}

Your heart tells you a distance function doesn't belong buried inside other code in your collision detector. Why? Its stateless. So it could be anywhere, and its reusable; maybe you'll use it somewhere else in CollisionDetector. Its not reusable otherwise yet, but that doesn't matter. Don't people say to make code usable before reusable? And so now the question in your mind is, how do I test that it's usable?

Make It Reusable

One of the reasons why testable code is so highly related to good code is that it enforces decoupling. To test your code, don't make the method public, make the method reusable.

We want our solution to keep our APIs clean and impossible to use wrong. And of course we can keep that. We can't call private methods of course, just like you can't call a public method on a private class member.

This gives us a simple pattern for preserving encapsulation, honoring the Single responsibility Principle, and making your private methods reusable.
  1. Create a new class to handle whatever responsibility that private method had. Suppose we're refactoring Recipe and creating a class Ingredient.
  2. Move any other Recipe code which belongs to the IngredienSet responsibility into the IngredientSet code.
  3. Give a new private member of type IngredientSet to Recipe

And that's it. Let me apply this to the simplest example:

Isolate pure functions from stateful classes
class DistanceCalculator {
    public long euclidianDistance(Point a, Point b) { ... }
}

class CollisionDetector {
    private DistanceCalculator distCalc = new DistanceCalculator();
    // some public methods...
}

By creating a class DistanceCalculator, we now have the ability to check distances anywhere in our code, including our tests. At the same time, we haven't polluted CollisionDetector's interface to include distance checking that isn't its responsibility.

Next up, why are we coupling CSV output directly in our StudentCsv class? Reducing code is good, but you missed the fact that you can:

Reduce responsibilities in a class
class Csv {
    public void write(File file, String... values) { ... }
}

class StudentCsv {
    private Csv = new Csv();
    // some public methods
}

It only took creating a CSV class to handle comma separation, and now anyone can quickly create a new report. Our student CSV only handles gathering the student information and feeding it into the CSV generator.

Finally, we're up against our alphabetized students listing. While making it testable is reward enough on its own, changing our implementation will also:

Actually ensure data coherence
class AlphabetizedList extends ArrayList<String> {
    @Override
    public void add(String s) {
        parent.add(s);
        alphabetize();
    }

    private void alphabetize() { ... } 
}

class Teacher {
     private AlphabetizedList students = new AlphabetizedList();
}

Here we created a subclass of ArrayList which imposes a limitation on its superclass. On each write, it alphabetizes itself. Now our Teacher class can simply treat its list of students as a list of students, and its impossible to have any code which changes the list and doesn't also alphabetize it.

If you're still actually reading my code examples, you noticed that something special about this last code example. It contained a private method, alphabetize() on AlphabetizedList!

When to Use Private Methods

This is where I reaffirm that private methods have a place, and describe gives alphabetize() the privilege.

Except, alphabetize() should not have been a private method. It was a trick. Alphabetize is an example of isolating pure functions from stateful classes. It is entirely conceivable that someone may want to one day alphabetize some other list. The fact that I made it private in the first place should be an immediate clue that the code might deserve a class of its own.

So when is it correct to use private methods, if ever? I could come up with many longwinded and vague, highly-nuanced answers, and it wouldn't help anyone at all. So I'll give you the practical, albeit informal answer instead.

Use private methods any time you already would have, and when you neither want to test, nor reuse the code.

Of course, if you probably will test/reuse it in the future, you might want to extract it into a class now before it gets ingrained into the system as a bad choice.

So next time someone says in a smarmy voice, "Why do you want to test a private method?" make sure to give them the answer.

No comments:

Post a Comment