Why shouldn’t I test private methods?

Newcomers to TDD ask some interesting questions, here’s one I was asked recently: testing private methods is bad, but why?

How did we get here?

If you’re trying to test private methods, you’re doing something wrong. You can’t get to TDD nirvana from here, you’re gonna have to go back.

It all started with an innocuous little class with an innocuous little method. It did one little job, had a nice little unit test to verify it did its thing correctly. All was right with the world. Then, I had to add an extra little piece of logic. I wrote a test for it, changed the class until the test passed. Happy place. Then I started refactoring. I realised my little method, with its handful of test cases was getting quite complicated, so I used the extract method refactoring and boom! I have a private method.

While simple when I extracted it, another couple of corner cases and this private method evolves into quite a complicated piece of code – which now I’m testing one level removed: I’m testing the overall functionality of my outer method, which indirectly tests the behaviour of the private method. At some point I’m going to hit a corner case that’s quite awkward to test from the outside, it’d be so much easier if I could just test the private method directly.

What not to do

Don’t use a test framework that let’s you test private methods. Good God, no. For the love of all that is right with the world step away from the keyboard.

What to do instead

This is a prime example of your tests speaking to you. They’re basically shouting at you. But what are they saying?

Your design stinks!

If you need to test a private method – what you’re doing wrong is design. Almost certainly, what you’ve identified in your private method is a whole new responsibility. If you think about it carefully, it probably isn’t anything to do with what your original class is. Although your original class might need renaming to make that obvious. That’s ok, too. That’s incremental design at work.

An example would help about now

Say I started off with a Portfolio class – it has a bunch of Assets in it, each of which has a Value. So I can implement a Portfolio.GetValue() to tell me how much it’s all worth. But then I start dealing with weird corner cases like opening or closing prices. And what do I mean by value, what I could sell it for, right now? Or perhaps there’s some foreign currency conversion to do, or penalty clauses for early exit, how does all that get factored in?

Before too long, GetValue() has a fairly large piece of logic, which I extract into GetSpotSalePrice(Asset). This method is then hairy enough to need testing, so it’s pretty clear that my design stinks. The deafening din of my tests, finally persuades me to extract GetSpotSalePrice(Asset) into another class, but here’s the million dollar question: which?

What not to do – part 2

For the love of SOLID, don’t put it in a AssetSalePriceCalculator, or a SalePriceManager. This is the number one easy mistake to make – you can follow TDD and ignore decent OO design and still end up with a steaming turd pile of horribleness.

NounVerber class is always a design smell. Just stop doing it. Now. I mean it. I’m watching you. I will hunt you down and feed you to the ogre of AbstractSingletonProxyFactoryBean.

What should I do then?

The answer might seem obvious, but to too many people starting out doing TDD and half-decent design – it isn’t at all obvious. The method needs to move to a class where that responsibility makes sense. In our example, it’s crushingly obvious this is really a method on Asset – it even passes one in. If your method has one class parameter and uses a bunch of data from that class, you can bet your bottom dollar you’re suffering feature envy. Sort your life out, apply the method move refactoring. Go live happily ever after.

Summary

Why shouldn’t you test private methods? Because the fact you’re asking the question means the method shouldn’t be private – it’s a piece of independent behaviour that warrants testing. The hard choice, the design decision, is where you stick it.

 

 

10 thoughts on “Why shouldn’t I test private methods?

  1. mrdfuse

    Can you share a bit more about the NounVerb anti-pattern? I just wrote a PriceCalculator at work😀 What if in your example the price calculation would need to check other Asset instances as well, so you can’t place the method in the Asset class. How would you solve that?

    1. Heh yeah this anti-pattern comes about for all the best reasons🙂

      My first question with something like a PriceCalculator is: does it have any state? If not, what you have is a method looking for a home. OO to me means you have state (immutable, ideally) with some messages you can send to that bundle of state to make something happen. Duck.Quack(), for example. Or Asset.Sell().

      A PriceCalculator sounds like it’s state is all external. It probably depends on Assets and Currency and ForeignExchangeCalculator and AbstractAssetPricingEngineFactory and and… I can’t tell you the answer without looking at your codebase (even then there’s no guarantee there’s an easy/obvious answer!)

      For example, maybe it’s in fact a Price object with some assets that it is the collective price of? Price.Calculate() feels more natural in that case.

      Depending on the domain, some other natural concept might fit better. As soon as your classes are in the *implementation* language and not the domain language, you’ve stopped modelling the real world and just started making shit up.

      1. mrdfuse

        Thanks for the reply. I’m gonna have to think some more about this subject🙂 Anyway, in my case the Calculator class does have some state. And we are starting with DDD in a legacy application of 12 years old…so it’s not always that easy to model everything correctly without breaking everything or having to change the entire codebase.

  2. Anonymous Coward

    I for myself find a very good but orthogonal reason for not testing private methods. You use unit tests to describe the desired contract of the class you test. Private methods are part of the implementation, not the contract. I don’t really care if they work or not. I just care for my class to do what I expect it to do – and don’t care _how_ it does it. Hence, there’s no business benefit in unit-testing private methods, and I won’t do it.

    In the context of a price calculator, you can look up the refactoring to a method object. A method object, in the right circumstances, is a perfectly valid class – which can be unit-tested thoroughly through its public interface.

    1. All quite true – the only reason to separate into another class is if you are unable (easily) to test interesting cases via the original public interface. The method object, to me, seems like a half-way house. You’ve extracted the logic to its own class, but it still describes what it *does* rather than what it *is*. That doesn’t mean it’s an invalid step, just I think another refactoring should follow.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s