BT

Facilitating the Spread of Knowledge and Innovation in Professional Software Development

Write for InfoQ

Topics

Choose your language

InfoQ Homepage News Private Methods, Test Driven Development, and Good Design

Private Methods, Test Driven Development, and Good Design

This item in japanese

Bookmarks

The claim has been made that test driven development (TDD) encourages good design.  The claim has also been made that TDD adversely affects architecture and design.  It helps to get a little more concrete than just discussing abstractions, so we will focus on private methods and their relationships to good design and testability - an instance of this apparent conflict.

Szczepan Faber blogged that private methods are an anti-pattern:

Private method a smell seemed to be ignited with the birth of TDD. Test-infected people wanted to know how to test their private methods. Gee… it’s not easy so the question evolved from ‘how‘ into ‘why‘: Why to test private method? Most of TDDers would answer instantly: don’t do it. Yet again TDD changed the way we craft software and re-evaluated the private methods :)

Jay Fields blogged on a generic way to test private methods in ruby:

... I rarely test private methods. I prefer to test through the public API. However, there are times when life is easier if you write a few tests for a private method or two.

Michael Feathers suggested last year in The Deep Synergy Between Testability and Good Design that TDD encourages good design and, conversely, code that is not testable should make us think twice:

When I write tests and I have the urge to test a private method, I take it as a hint.   The hint tells me that my class is encapsulating so much that it has ceased to be "understandable" by tests through its public interface.  I listen to the hint, and factor my design differently.  Usually, I end up moving the private method (and possibly some methods around it) to a new class where it can be non-private and accessible to tests.

All of the ideas above tend to discourage the idea of private methods and put more weight on testability.  But they are not the only word on the subject.  In fact, much of what has been written about object oriented design encourages as much encapsulation as possible and fewer classes.  By only exposing the absolute minimum in a public API coupling is minimized.  David West, in Object Thinking, sites Lorenz and Kidd in Object Oriented Software Metrics:

  • An application should consist of no more than 40 stories and no more than 100 classes.
  • The application's entire business domain should not require more than 1000 classes.
  • 25-30% of the code should be discarded after each iteration.
  • Responsibilities per class: average of 7.
  • Methods per class: average of 12.
  • Lines of code per method: 15.
  • Percentage of lines of code requiring comments: 60.
  • Number of case statements: average of 0.

If private methods are a smell and need to be pulled out into their own classes aren't we increasing the number of classes in our application just to make testing easy?  It seems that doing so will quickly get us to a very large number of classes.

So, what about private methods?  They are annoying to test.  Do we change private methods and expose them for testing?  Do we decide not to test private methods and keep design independent of testability?  Or are private methods a code smell - an indication of a class trying to do too much?

Rate this Article

Adoption
Style

Hello stranger!

You need to Register an InfoQ account or or login to post comments. But there's so much more behind being registered.

Get the most out of the InfoQ experience.

Allowed html: a,b,br,blockquote,i,li,pre,u,ul,p

Community comments

  • No Idea

    by Li Hongchao,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Re: No Idea

    by Sai Venkatakrishnan,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • package private?

    by Kristian Marinkovic,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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).

  • 60% of the code needs comments??

    by Eric Lefevre-Ardant,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Just stupid

    by Martin Vanek,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Re: package private?

    by Martyn Roberts,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Balance

    by Dan Thiffault,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Guidelines

    by Bruce Rennie,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Dogmatic Fundamentalism a code smell?

    by gcom nz,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Re: No Idea

    by Erick Dovale,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Re: Dogmatic Fundamentalism a code smell?

    by Amr Elssamadisy,

    Your message is awaiting moderation. Thank you for participating in the discussion.


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

    - 39 words
    - 48 characters
    - 5 lines


    LoL, you have a point! But, to be fair to the quote of the quote of the .... Those numbers were derived from existing systems. And, as Bruce commented above, these are guidelines - i.e. suggestions.

  • (i.e. my real opinion)

    by Amr Elssamadisy,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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?)

  • Are you testing behavior or implementation?

    by Javid Jamae,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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).

  • Re: (i.e. my real opinion)

    by Jon Weaver,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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

  • Re: Dogmatic Fundamentalism a code smell?

    by gcom nz,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

    :-)

  • Re: Are you testing behavior or implementation?

    by gcom nz,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    @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.

  • Re: Just stupid

    by Amr Elssamadisy,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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...."

  • Re: Dogmatic Fundamentalism a code smell?

    by Amr Elssamadisy,

    Your message is awaiting moderation. Thank you for participating in the discussion.


    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 certainly meant no disservice to the community by posting this article. It is a point of view that I have heard from several people that I respect. I don't necessarily agree with it (ok, actually I don't), but I thought it was worth airing and discussing.

    Amr

  • Re: (i.e. my real opinion)

    by Amr Elssamadisy,

    Your message is awaiting moderation. Thank you for participating in the discussion.


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


    Actually it is not my idea, I read it in a paper published in the early days of the XP conferences (around 2003) by Eric Nickell and Ian Smith (of PARC - wow, I didn't know they still did cool stuff!). It goes something like this:

    1) You create an inner class within the class under test - call it TestProbe
    2) You create an accessor to that probe in the parent class - something like getTestProbe()
    3) Because the inner class has access to all the private methods and variables, you can add as many getters/setters as you want to fudge with the inner state of the class under test.
    4) You get to keep the parent class's original private variables and methods and minimally modify it's public interface by adding one getter: getTestProbe().

    An example of a TestProbe might look something like this:

    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);
    }
    }
    ...
    }

  • Re: Just stupid

    by Dave Rooney,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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

  • Re: Just stupid

    by Al Tenhundfeld,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • The Smell Is Not Private Methods Per Se But Those Needing Testing

    by Slade Stewart,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • The most bonkers "story" ever.

    by James Richardson,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • you should not write private method at first

    by lau vincent,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Re: (i.e. my real opinion)

    by David Nunn,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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!

  • Not all privates are evil

    by Jason Royals,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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%.

  • Re: The most bonkers "story" ever.

    by Amr Elssamadisy,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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 ;) )

  • Re: (i.e. my real opinion)

    by Amr Elssamadisy,

    Your message is awaiting moderation. Thank you for participating in the discussion.


    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?


    Dave, you seem to have misunderstood TestProbe. It is not the comment that protects you, it is the fact that you have minimally changed the public interface - you have added one method.

    So, for someone to access the private data in the class they have to do something like: MyClass.getTestProbe().getPrivateMethod()

    Unless you totally distrust your developers, the fact that you are calling MyClass.getTestProbe() in production code never happens. Now, if you think your developers are idiots or malicious, you have a much bigger problem than private methods ;)

  • Re: Not all privates are evil

    by Amr Elssamadisy,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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?

  • Re: Not all privates are evil

    by Declan Whelan,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

  • Ports & adapters is the answer

    by Daniel Gruszczyk,

    Your message is awaiting moderation. Thank you for participating in the discussion.

    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.

Allowed html: a,b,br,blockquote,i,li,pre,u,ul,p

Allowed html: a,b,br,blockquote,i,li,pre,u,ul,p

BT