The Inquisitive Coder - Davy Brion’s Blog

I got a question from one of my readers about how to properly test private methods. She’s new to TDD and she’s not always clear yet about the best ways to test certain things. She sent me an example situation and asked me how she should properly test it.

I think the only way to properly test private methods is to not test them directly. We’re talking about private methods. They are private. Outside of the class they belong to, they are nobody’s business. They don’t even exist, as far as the rest of the code is concerned. They are basically just an implementation detail, and ideally, your tests should know as little as possible about implementation details.

Some people don’t see a problem with testing private methods, and as such they often propose to access the private methods through reflection, or to even just make them public. If you’re dealing with legacy code that you just can’t transform to a proper design in one go, then one of these approaches might be a satisfactory temporary solution in order to get some kind of test harness in place. But if you’re writing new code, there are definitely better solutions available.

Let’s think about the concept of testing a private method. If you feel the need to test a private method (which is always a part of a larger picture: some kind of public method which uses the private method among other things), then it often means that the private method is simply doing too much. If this private method uses other private methods, it’s probably a class just waiting to be discovered. After all, if the functionality provided by the private method were small, then you would’ve never thought of testing it separately, right? You would’ve been able to completely cover its functionality through the tests of the calling public method.

Let’s take a look at the following example. The example that my reader gave me was an import of an excel sheet which contains hierarchical data. Suppose the data looks like this:

Obviously, this isn’t real data and it’s a very simplified example (”no shit, Sherlock!”). We have to write code which parses this data and creates proper entities from this: customer objects with orders, which in turn contain orderlines which in turn reference a product. So we need some kind of Parser class, right? The Parser class could have one public Parse method, which would loop through the rows and the columns. Within the loop, it would need to determine what kind of data is contained in the current row and it needs to create an object of the right type for the data in the current row. For each row, it also needs to determine if the rows beneath it are children of the current object. Each child could in turn also have other children. Nobody in their right mind would have all of this code in our public Parse method. So we make use of private methods to deal with all of the possibilities.

How would we test this? Well, we could set up a test case which processes the data shown above, and verify that the objects that were created by the code have the correct values, according to the data we’ve provided to the Parse method. But we wrote all of these private methods, which sometimes need to deal with a lot of possible corner cases as well. We could write a bunch of tests for our public Parse method and make sure all of the corner cases for each possible type of row (customer, order, orderline) are properly covered. You could do that, but it requires you to set up a larger data set than you might need to test the specific corner cases. As the complexity or your data structures grow, so does the complexity of your tests. Well, with this approach anyways. There must be a better way…

Let’s take a step back. What is the responsibility of the Parser class? Well that’s easy… it needs to parse the data! But that’s a bit of a general statement, and we should try to split this up. In this case, there are at least 3 separate responsibilities: the parsing of customer rows, the parsing of order rows, and the parsing of orderline rows. One Parser class to do all of this? Wouldn’t it be better if we use separate Parser classes?

Here’s what i would do: i’d have one ExcelParser class which is responsible for opening the excel sheet, and reading the data from it. As it loops through the rows, it needs to detect which type of data is contained in the current row, and then instead of calling a private method, it just calls a public method of a specific Parser class which is suited to handle the current kind of data, and passes the data of the current row into it. This could be a CustomerParser, or an OrderParser, or an OrderLineParser. Depending on which kind of specific Parser that is needed, you’d probably also have to pass the data from the previous row as well (which would be the parent object). The ExcelParser class would no longer contain all of the code to parse every possible piece of data. It just gets the data, loops through it, and calls the proper specific parser. That’s it.

So how would we test that? Our first test is a good start. We’d simply pass the entire data structure (pictured above) into the ExcelParser and then we’d verify that the correct objects were created with the proper values. So far, it’s the same story as for the first implementation. But now, we can test all of the corner cases in isolation by directly testing the specific parsers. We need to do something tricky while parsing an order? No problem, write a test for the OrderParser’s public Parse method and simply pass in a row containing the data for an Order. Want to test something specific to how the retrieval of an OrderLine’s Product reference is set up correctly? No problem, write a test for the OrderLine’s public Parse method and simply pass in a row containing the data for the orderline (which contains a reference to a Product). I think you get the idea.

It might have been better if i had written some code to demonstrate this approach, but i didn’t have enough time for it and i guess the approach is pretty clear from the text. I think this example clearly shows why it’s sometimes better to just split your code into smaller, more focused classes instead of trying to do too much in a single class. Even if at first sight the single class is taking care of a single responsibility. If you end up with a lot of private methods, it usually isn’t.

And for those of you who’re thinking “oh no, that leads to a shitload of small classes and more files”, you could just as easily put some of the small classes into the same file. The file probably won’t end up being larger than it would have if it only had the single big class in it.

Anyways, this is all just my opinion on how i would approach this, and it’s actually my ’standard’ approach whenever i get the feeling that the code in a private method should be tested in isolation. I’m sure some of you would disagree or have other solutions. And i’d love to hear about it, so don’t be afraid to leave a comment