Private Methods, Test Driven Development, and Good Design

OK. I must commit that I was confused. I intend to not writing any unit test code for private method. Because I can prove my application works through unit test code for public interface.

Yes you can prove that private methods in your application work through the API exposed as public. But the real problem is when your private methods become bloated and carry too much responsibility. At this time it will not be very convinient for any TDD guy to it as such. Thats when the real need for testing private methods rise. The better soltuion according to me would be to refactor and make your design better though it may increase the number of classes. At least they will have their responsibilities distributted in a better way.

if you really want to test private methods.... (i don't).
you could change your private methods to package private. so they are available only in your test classes (and the classes of the package).

I thought that the ideal of a project was to minimize comments by restructuring the code?

I am also confused by the goal of 100 classes maximum. Even discounting interfaces and classes dedicated to test code, it still seems very low to me.

Mark private method with smelly label, just because they are not directly runnable from unit test, makes author of that statement at least ignorant, period.

I think it is a failing of the language if it does not natively support a simple way of allowing test code to access private methods in classes under test. Until your language supports this you are forced use reflection based workarounds or, as just suggested, make your private methods package private.

Private methods aren't a code smell, trying to test them is though. A classes' responsibilities should be succinct and solely expressed through its public api. So their shouldn't be a reason to want to test the private methods of a well written class.

It is tempting to test private methods when they encapsulate so much logic that to test their behavior an abstraction level higher (behind the public method(s) which call them) is infeasible. Only when you get to this point should you factor out the private method into its own class.

I think the biggest problem in these discussions is that developers don't understand the meaning of the word "guidelines".

Some developers, who may or may not know something you don't, put forward some guidelines for what they consider to be "good". You don't have to agree with all of them. I don't even think you're supposed to. That's not the point. The point is that when we're writing code we're supposed to continually ask ourselves "does this make sense?". Guidelines help provide some framework for that evaluation. Guidelines, even more than rules, are meant to be broken, however. It's not that you always comply with the guidelines, it's the fact that you reviewed your code in the first place.

In that spirit, I absolutely agree that private methods could be a source of concern in any given class. I agree with the guideline that I should not test those methods. I agree that, if I should find myself really needing to test a private method that there is probably something wrong. And I might go ahead and ignore the guidelines and write a unit test for a private method anyway and walk away happy in the knowledge that I've done what I had to.

This is awfully dogmatic. Here's a thought, testing is private, by nature, so maybe they just need to fix their testing libraries to bypass the restrictions on calling private methods?

Better yet, how about posting an article about the dangers of dogmatic fundamentalist my-way-or-the-highway testing code smells ;-)

Oops, my comment went over the optimum for a blog comment:

- 39 words
- 48 characters
- 5 lines

I'll get back to my rationed method lengths now, sir.

I think a statement like no more private methods is just plain wrong. In my TDD experience I have noticed that there is nothing wrong with private methods as long as they are not dealing with a foreign concern. By foreign concern I mean a concern that violates the SRP. That's when refactoring the method into its own class starts to make sense. Testing through the public interface is, IMHO, the way to go though.

Oops, my comment went over the optimum for a blog comment:

- 39 words
- 48 characters
- 5 lines

I usually find myself holding back my own opinion when writing a news article. So here are my thoughts:

1) We need to agree what good design is before we can argue about it ;)
2) IMHO good design is anything that will allow us to meet and respond to our customer needs better. In that sense testing is important, but so is coupling and cohesion, understandability, mirroring the real world with our designs, etc....
3) Saying that private methods are bad because they don't allow tests just doesn't hold water. Where do things like sorting, traversal, algorithms, and all the calculations go? If we pull them out into their own classes we are making something public that doesn't need to be - i.e. we are increasing coupling. etc...

So, I may be getting old, but when I need to test a private method (and many times I do) I don't pull it out into its own class and I don't change it to protected. I keep it private and use something called a "Test Probe" that works really well. (How's that for a cliffhanger?)

Let's get one thing straight, you cannot test a private method unless the test is in the class with the private method itself. You have to change the visibility of a method to test it from a separate class. As others have said, if you're trying to figure out how to make something public just to test it, you should take a step back and think about what you're doing.

But on the same token, using the visibility of a method as the litmus test for whether you should test a method or not is equally problematic. If visibility were the only criteria, then I could just make everything non-private and justify testing it. Consider the case where you extract a private method into a new class and make it public. Is it all the sudden worthy of testing? Isn't it the same code that was not worthy of testing just a second ago?

I think the key here is to think about whether you are testing behavior or implementation. I don't want my tests to start failing just because I changed an implementation detail (i.e. did a refactoring).

I find myself much more in agreement with this set of statements. Good design must be defined before we can claim something like private methods makes for bad design.

Testing should focus on determining if the contract/design meets the requirements not on does the implementation allow me to automate testing at the most granular level. Focus should be on the public api.

Private methods may best be tested by the developer working toward meeting the TDD contract using a debugger. I am interested in hearing more about your "Test Probe".

Jon Weaver
XAware.org

Yeah, I was being extreme. I actually feel that the author of this article did a bit of a disservice to the source authors by making it seem like the original articles were unbalanced. Judging from other comments, I don't think I'm alone in that reaction.

And of course, I believe that the practices suggested by some of the authors are very good to integrate, and the original question regarding private methods, is a very interesting one. I happen to agree that too many private methods probably is a code smell in many cases, but not the concept in general.

Actually, to add to the discussion, perhaps API writers should consider that, instead of making private methods as much, they could create an internal API for their own library's use. I haven't thought too deeply on that, but it's something I want to mull over a bit more.

:-)

@Javid: Not that I'm clear whether its a good practice or not, but it's really quite trivial to bypass the access restrictions on private methods, and is done by plenty of libraries.

Stupid aside.... Some pretty smart people are saying this (not me - on both counts). I think the argument goes something like:

"If I can't test it then what's it doing in this class? Is the class trying to do too much? Probably so, and I need to test anyway...."

I actually feel that the author of this article did a bit of a disservice to the source authors by making it seem like the original articles were unbalanced.

I am interested in hearing more about your "Test Probe".

public class Foo {
private Map cache;
private int itemsInCacheMatching(String pattern) { ... }

/** For tests only. NOT TO BE USED BY PRODUCTION CODE. */
public class TestProbe {
public Map cache() { return cache; }
public int itemsInCacheMatching(String pattern) {
return Foo.this.itemsInCacheMatching(pattern);
}
}
...
}

As always in software development, "it depends". ;) Private methods are fine when they are small, don't duplicate other code and can be tested through the public API for a class. What is problematic is the Iceberg Class smell, where a small public API hides a mountain of private code.

I've also had plenty of people ask the same question about testing private methods, and I always answer them with a question: Why is the method private? In some cases it deserves to be, but in many cases the method doesn't even belong in the class! My question is intended to at least provoke some thought on the part of the person asking.

Dave Rooney
Mayford Technologies

I'm a little confused by this whole discussion of testing private methods. In the .NET world, testing private methods is extremely easy through generated accessors, which use reflection to expose private methods & attributes to your testing environment.

And to address some of your points in particular, I thought the point of a public API was to abstract and simplify interaction with a system and thus hide a mountain of private code. For example, if I want to use a data access library to talk to a database, I just want to have a public method that lets me open a connection. I don't want to know about all of the communication handshakes going on under the hood. That's why I'm using the data access library instead of writing my own.

Furthermore, I often use private methods to break complex methods down into more testable, intelligible chunks. For example, I may have three private methods that respectively open a file handle, read the contents of the file, and then close the file handle. But I just want one public ReadFile function. If anything, I think the judicious use of private methods facilitates unit testing and aids in separation of concerns.

If your unit testing framework can't test private methods, then it's time to find a new framework. If it's impossible to accomplish in your chosen language, then it's time to start helping your language of choice mature to a point where it can support this feature.

This post IMNO captures the crux of the issue and a disparity in concepts between different posters. However I thought it needed a title that lets people know what it's saying.

Either the story was posted 96 days early, or this website has gone the way of theserverside,and the comments the way of slashdot.

One way of thinking:
Private methods are already tested, by testing the functionality that the class exposes through its public api.

Another way of thinking:
- Twiddle with reflection to expose private methods to test them. Or make them package private
- Add a "TestProbe" to production code to expose all private data
- Impose arbitrary limit on number of classes allowed per "compliant" application.
- Its a language failing to allow private methods / data.

Its "stories" and comments like these that lend weight to the hypothesis that agile software developers are clueless bandwagonesque fanboys looking for the next bit of self-congratulatory pseudo-intellectual IT nonsense.

goodness.

I think private method just the one of lots of result with refactoring,if public method can pass unittest,then extract some code to private method which is no necessary to be tested.re-test that public method it's ok.

The whole context of "good design decision" really has nothing to do with the code as it stands now or in the next 6 months, it is what it does to your successors 5-10 years down the line. The costs of coding up the current release are small compared to the costs of maintaining it (for most software).

So instead of thinking about what you are coding, think about the code you've maintained written by other people no longer able to explain themselves because they've retired, moved on, etc.

My experience has been that other people's past "private" decisions almost always provide me a great deal of apparently needless pain when I set out to maintain it.

About the only use I really see for "private" is to prevent polymorphism. Other than that, for development purposes, if I'm mucking about in the code, I can simply remove the private tag (or change it to protected if I'm sure that overriding won't break anything).

The Test Probe solution mocks the whole idea of private. Do you think the comment will stop someone 3 years later from using that (if they understand it) to access your private parts? It does make the code more complex (so if you're going for job security it's an idea), but it's not as complex as just using reflection everywhere (highly recommended for job security).


Be kind to your heirs, and make the methods protected (package + subclasses) or package. You should have a compelling reason to make them private!

The way I see it, a private method is an artifact from refactoring to remove internal duplication and improve readability, and that's about it. I don't think it's evil, but I do agree that the desire to unit test a private method is a smell. If you're doing TDD properly, you should not have any private methods until the test goes green. Only then should you refactor what you need to. That makes any extracted private methods 100% unit tested through the public API.

Before my test driving days, I'd sometimes get complex private methods I'd want to write a test for. I firmly believe now that desire is a clear message that the private method I wanted to test was actually a new class begging to be written, then injected in wherever it was needed.

Sure, you'll end up with a lot more classes, but is that really a bad thing? Someone once said "I'd rather have a lot of simple classes working together, than have a few really complicated ones". Having worked on several enterprise applications at both ends of the scale, I agree with that statement 100%.

Ok, I'll bite.... What's wrong with the story and why does it show that people in the Agile camp are crazy?

1) Do you have private methods in your code? Should you have private methods in your code? (Sounds like a reasonable enough question.)

2) Do you test your code? If so, do you test your private methods? If not, why not? (Yep, the most ridiculous question I've ever seen in my life ;) )

The Test Probe solution mocks the whole idea of private. Do you think the comment will stop someone 3 years later from using that (if they understand it) to access your private parts?

I agree....

Let's assume that you have a class that does something. That something is done in more than one method in the class. So you remove the duplication by extracting method. Should that method be *anything* but private? If so, why? And, now that you have it, don't you want to test it?

I think Slade Stewart has it right - "The Smell Is Not Private Methods Per Se But Those Needing Testing".

The point is that if you are doing TDD and you extract a method then the tests on the original public methods are still implicitly testing the extracted private method. There is no need to do further explicit testing on the private method.

Testing private methods is a bad practice in theory because it breaks OO encapsulation. It is a bad practice in practice because it adds redundant test code which is a liability that will impair future refactoring.

If you are testing your private methods, you are testing your IMPLEMENTATION. You should always test your REQUIREMENTS as a black box, and make sure you are not influenced in your tests by your implementation.
Have a comprehensive set of black-box tests around your requirements. This way when you change your implementation, but your requirements don't change, you don't have to change your tests.
By any means, you can TDD any complicated classes in your implementation, but for your own sake, throw these tests away once you get your black-box tests passing. This will make your life easier, development faster, and maintenance of tests easier.
Ports & adapters (or hexagonal architecture) is a great place to start, and changes the whole process of designing and developing a new piece of software.