No private methods

Published 11 June 04 05:54 PM

I’m heading off into the deep end here.  If you want to come with me, make sure you’re wearing your SCUBA gear.

 

One of the first questions that come up in TDD with NUnit is “How do I test private methods?”  Usually the answer looks like “Don’t do that, because XXX”, like:

 

·         If your class has interesting private functionality that you’re inclined to test, that functionality might be interesting to a consumer of your class.  Expose it.  In this way, TDD is an exercise in Domain-Driven Design, where the domain is software writing, and the user is you in a few minutes.

·         It’s a code smell.  If your class is so complex that you need to test private methods, you should refactor to pull some of that complexity into its own class.

 

There are other reasons, I’m sure.  If you have one, comment on it…

 

So I had this crazy idea.  What happens if you take the second bullet to the logical extreme?  What if you continue performing Extract Class until there are no more private methods?  Private fields are fine, but only public methods.

 

It’s an idea.  I don’t know if it’s a good idea or a bad idea.  Even if it’s a good idea, I don’t know if I can do it well.  So if I try it and the result sucks, I don’t know if it’s because the idea is bad or just because I can’t do it right.

 

I can try to imagine what might happen if you try this.

 

Obviously, you’d have a lot of small classes.  You’d need to be careful to ensure that they made sense.

 

I think you would end up with deep nesting of classes, with built-in types at the very bottom, like:

 

      class C

      {

            int i;

 

            public void F() { }

      }

 

      class D

      {

            C c;

 

            public void G() { }

      }

 

      class E

      {

            D d;

 

            public void H() { }

      }

 

How does this jive with Object Thinking’s suggestion that you shouldn’t end up with a large number of classes?  (I think it does jive, but I need to think about it more.)

 

Is the result really easy to unit test?  I hope so, but is it?

 

Is the code ultra-readable or a total mess? 

 

Is refactoring this code impossible or trivial?

 

If you get good at this, does it change your thinking about the ideal size of a class?  (The balance between many simple classes and few internally complex ones?)

 

I need to find a good programming problem & try this out.

 

Comments

# Klaus Probst [MVP] said on June 11, 2004 6:17 PM:
But the question is bogus to begin with. Private methods should exist only in support of exposed entry points and public aspects or facets in your components. There should be *absolutely* no requirement that involves even remotely trying to get to them. A good testing strategy should cover them completely, since they'll be excercised as the public interface is put through its paces.
# Karl said on June 11, 2004 7:54 PM:
aside from the debate about whether private methods should be tested, any thoughts on this solution for testing them:


#if UNIT_TEST
public static ArrayList GetCategories_UnitTest (string filename){
return GetCategories(filename);
}
#endif
private static ArrayList GetCategories (string filename){
//implementation
}
# Jonathan Cogley said on June 11, 2004 8:04 PM:
I get asked this question frequently when presenting on TDD - in fact I even have a slide on it! I tend to agree with what Klaus said. Don't think about testing every method, rather focus on testing features. Remember that TDD comes from XP where business value is key ... therefore each test should be focusing on an aspect of business value and all code supporting it will then have full coverage.

Your "Extract Class" idea is interesting but your design should be dictated by refactoring after getting to green rather than to test each method. Think "tests == features"!

# Adi Robinson said on June 12, 2004 2:44 AM:
I your idea because it's so controversial, from an O-O perspective. On the extract class refactoring I have to say that object domain pollution and loss of cohesiveness in the abtractions being modeled are good reasons to refute it...however command pattern and the strategy pattern represent approaches where externalising a feature can have some design benefits.

However, from a TDD perspective I think we have to decide on one of two approaches.

1) Do I only test my public interface - and thereby if my public interfaces honours it's contract the discrete private implementations MUST work (at least within the bounds of the values passed to them from the public interface, AND assuming exhaustive public interface testing)

or

2) Do we test private methods because as stand alone functional units they have pre and post invariants I want to force to be correct?

You could argue, that if you've done proper TDD, then your private methods emerged as extract method refactorings once duplication occurred and so hence where tested as part of that design evolution? And, given that you would clearly re-run your old tests on the now multi-method-provided implementation of a once contiguous code block means that if you get the green light you are indeed testing your private methods.

I'm not entirely sure either way.
# Nicole Calinoiu said on June 12, 2004 8:04 AM:
There's another potential consequence that you didn't mention: impact on the performance/security trade-off.

Ignoring for the moment the potential impact of reflection into low accessibility members, a private method sits inside the input boundary of its class. For most private methods, very little parameter validation is necessary since this has already been performed elsewhere in the class or the values were safely generated inside the same class. However, if the method becomes a public method of another class, all parameters must be re-validated completely since their values are supplied from outside the input boundary of the new class. In general, a performance decline should be expected, particularly if one is calling the method from within a loop.

Obviously, it is possible for callers outside a given class to be considered trustworthy, so the above is a bit of an over-simplification. However, the general problem applies any time you open a low accessibility member to more callers.

That said, if reflection into low accessibility members is a concern, the method is probably already written as if it were public, so the performance impact of the change isn't likely to be a consideration.
# Michael Swanson said on June 12, 2004 8:59 AM:
And it's "jibe", not "jive". :)
# jaybaz [MS] WebLog said on June 14, 2004 12:39 AM:
# Mikhail Arkhipov (MSFT) said on June 13, 2004 11:14 PM:
Programming reflects real life. 'No private methods' is like 'no private life' :-) If someone is calling you and asking for something you have right to talk to your significant other before answering. I believe every object has right to the private life as well :-).

Seariusly, exposing everything increases test matrix. If before a private method A was only supposed to be called from B now anyone can call A at any time.
# jaybaz [MS] said on June 14, 2004 11:21 AM:
Michael: You're right! A goggle search on "define: jive" didn't mention compatibility. A search on "define: jibe" was mostly about sailing (yay) but also had this entry at the very bottom.

"be compatible, similar or consistent; coincide in their characteristics"

http://www.cogsci.princeton.edu/cgi-bin/webwn?stage=1&word=jibe
# Darren Oakey said on June 14, 2004 6:50 PM:
I know you are just throwing up a question, what about "no private methods" - but I'd like to squash it :)

I always make an attempt to make as much of my underlying implementation as private as is theoretically possible, and think that's a good thing..

I think almost all the advances in modern languages and concepts come down to information hiding - or, to look at it another way - "simplifying".

In the bad old days, I had to understand every bit of making an ftp connection, listening on a port, worrying about synchronisation, etc etc etc.

Now I just go "new FtpConnection( server)", and myConnection.GetFile(). Over time, with many people using the same library, in the same way, we find the bugs - it gets more and more robust - deals with more and more situations, and the code gradually improves.

This occurs because we have a tiny interface as wrapper to a large amount of functionality. Even though it may sometimes be restrictive, people always use it the same way, because that's the only choice they have.

And that's how software improves. Expose all the underlying interface, allow people to change things, and we've just stepped back 20 years into unproductiveness...

Anyone who has programmed for a long time knows that Limitations = productivity. (Which is why, as someone who's experienced in everything from asm to vba, I'm more likely to write a quick program as a perl script than a beautiful c++ structure - because it'll take me 1/10th the time, and be 100% stable)








# Travis said on June 14, 2004 8:30 PM:
One positive side effect might be amazing code reusability.
# Travis said on June 14, 2004 8:48 PM:
Trackback http://travis.pettijohn.com/blog/comments.php?entry=98
# jaybaz [MS] said on June 14, 2004 9:05 PM:
Travis: I hope so. Let's try it & see.
# jaybaz [MS] said on June 14, 2004 9:08 PM:
Darrel: I think your concern sounds valid. But I hope that the external interface doesn't charge, so things work out ok
# James Newkirk's Blog said on June 16, 2004 8:59 PM:
# James Newkirk's Blog said on June 16, 2004 9:05 PM:
# Iain said on June 17, 2004 8:08 AM:
I think it's kind of self-evident that the private methods should be tested, because unit testing seems to me to be about testing the smallest testable pieces, as opposed to testing generally, which seems to be about testing all the big pieces.

This makes it a question of mechanics. The really obvious solution is to have a compiler option that makes the compiler spit out reflection-based invocation code for private functions, so you just write your tests for private members like you do for everything else and pass the violate-encapsulation switch to the compiler, or somehow indicate it otherwise.

I might have to try implementing this with the mono c# compiler.

It would be possible to just write the reflection invocations for private members yourself, but I think that's just going to make the tests harder to read by a large enough margin that it's going to be a pain.

The violate-encapsulation would be subject to abuse, but I can't see it being any worse then the ability to reflect on and therefore invoke private methods when in full trust already is.
# public MattBerther : ISerializable said on June 23, 2004 2:42 PM:
One of my favorite bloggers as of late is Jay Bazuzi (rss), who works on the Visual C# IDE team at Microsoft. Give him a read; he's got some posts that are sure to melt your brain, such as "no...
# jaybaz [MS] WebLog said on August 19, 2004 6:17 PM:
# is private method an antipattern « monkey island said on January 5, 2008 2:30 PM:

PingBack from http://szczepiq.wordpress.com/2008/01/05/private-method-antipattern/

New Comments to this post are disabled
Page view tracker