Unit testing private methods yes or no?
I am beginner at field TDD. Recently I have conversation with Dror Helper should I test my private methods or not. After I have talked with him and little reconsider I thing we don’t need to test private methods.
If we need to test private methods we can do this trough public methods that call those private methods.
For example if we have one public method that calls one or several other private methods like this:
public static bool LogIn(string userName, string password)
{
var user = new User(userName, password);
if (verifyThatUserIsValid(user))
{
return true;
}
else
{
return false;
}
}
private static bool verifyThatUserIsValid(User user)
{
throw new NotImplementedException();
}
So if I want to make unit tests for method verifyThatUserIsValid I can easily make unit tests for method LogIn to test that method. What do you think about this?
If you want to learn more about unit testing I recommend a great book that I love written by real expert:
Jim R. Wilson
April 11, 2009 at 8:28 pm (15 years ago)Hi Radenko,
I’d recommend not unit testing private methods. Since they’re private, they may be changed in any conceivable (or inconceivable?) manner between each release. If a given private method is so critical to the operation of the class that you feel it deserves test cases, then it’s probably time to refactor that out into a protected or public method.
Generally speaking I think that private methods hamper code reusability, so any time you can make something public (or at worst protected), IMO that’s a better route to pursue.
The purpose of a test case, as far as I understand, is to prove the API for a given method. That is, given a method, a test case exists to prove that for a given set of inputs, a given output can be expected and counted on. From that perspective, a private method has no "API" insofar as no other class will be able to get at it, so making a test case doesn’t really make sense.
Anyway, that’s my $0.02. Hope it helps!
Radenko Zec
April 11, 2009 at 10:15 pm (15 years ago)Hi Jim.
Thanks for comment.I share the same opinion.
BlackWasp
April 11, 2009 at 10:22 pm (15 years ago)You have stumbled upon one of those questions that often causes a shouting match 🙂
Personally I try to avoid testing private methods, except of course for thorough testing via the public interface. Sometimes though it makes sense to test some private methods if they perform a particularly complex task but one that you do not wish to be accessible directly from outside of the class. I believe that we shoudl strive for the "correct" answer but that sometimes we just have to be pramatic about these things.
Michael Merino
April 11, 2009 at 10:57 pm (15 years ago)Your blog post was exactly what I would write if I were to have the same subject line. The basic concept is to test the entire public interface. By completely testing your public interface, you’ll have accessed the appropriate parts of your private implementation.
I can imagine a few cases where you might, on a risk-adjusted basis, test a certain algorithm. But that is why your blog is a good guideline to follow.
Radenko Zec
April 11, 2009 at 11:55 pm (15 years ago)Thanks for your valuable comments.
I am beginner in field TDD and what is interesting there is not very much literature for TDD with C#.
Ryan Stewart
April 11, 2009 at 11:56 pm (15 years ago)I agree: no testing of private methods, but for different reasons.
First consider what’s being said. We test to verify the correctness of software, so by saying that we don’t test private methods, we’re tying verification to method visibility and saying that regardless of their content, we don’t care about the correctness of private methods just because they’re private. That’s a rather arbitrary connection. Private methods form a critical part of a class’ "internal API" on which the class depends to function correctly. It’s vital, therefore, that the private methods are correct, or everything built on top of them will be broken as well. Further, the argument that a private method shouldn’t be tested because it’s not part of an API is specious and dangerous for the same reason. With all of that said, it seems obvious that we can’t reject the testing of private methods simply because they’re private, so why do I say, "No testing of private methods"?
Regardless of visibility, would you test a method that had just one or two simple lines? Probably not, unless you’re trying to beef up your test coverage numbers. When you look at it in this light, it’s obvious that we choose whether or not to test a method based on its complexity, not its visibility. Unfortunately, there’s no clear measurement of when a method is no longer "simple". I’d say if you can’t look at it and see all the possible outcomes in 5 seconds or less, write a test for it.
My reason for not testing private methods is that it’s messy. They’re private. They aren’t meant to be accessed by anything else. That makes them inherently more difficult to test than methods of other visibility. Now, if I want to test based on complexity (I do), but I don’t want to test private methods (I don’t), then what? It becomes obvious that I can’t allow my private methods to be anything but simple. Of course, as you start on new features, and as code evolves and requirements change, private methods that started as simple one- or two-liners may become significantly more complex. What to do then? Here’s my suggestion, though from a Java standpoint:
http://shotgunsandpenguins.blogspot.com/2009/01/good-design-in-42-seconds.html
Emmanuel Caradec
April 12, 2009 at 12:08 am (15 years ago)After a while programming tdd, I went to the fact that private methods aren’t required. That’s what solved this issue for myself.
If you write private methods, may be they don’t need to be private : if they are no potential to broke the object internal and are useful, they may as well be public.
Many private methods are considered a smell too and can be refactored as an object with a subset of data from the original object.
I tend to think private members as a commodity, a sign of imperfect design. That’s not a issue usually as imperfect is good enough most of the time.
C2.com has a pages about testing public members :
http://c2.com/cgi/wiki?UnitTestingNonPublicMemberFunctions
and about having only public methods :
http://c2.com/cgi/wiki?MethodsShouldBePublic
Radenko Zec
April 12, 2009 at 12:32 am (15 years ago)I think that I have a habit to make every method that I can private.
Hm but now when I beginning with TDD I must get rid of that bad habit 🙂
But when I think if we make all methods public that maybe also wrong?
I can write very long public methods or broke this methods into several smaller logical methods but I don’t want those smaller methods to be public.
I want them to be private but I can test them though writing tests to public methods witch contains private method calls.
BlackWasp
April 12, 2009 at 2:32 am (15 years ago)I wouldn’t throw away the idea of having private methods just because they are difficult to test. Nor would I suggest that every method should be public. I think that this is a debate that is far from won (and I hope won’t be won by the public-only side).
There are so many things I find wrong with the principle that I can’t list them all here. Here are a few:
1. Making everything public, and then testing everything, means that you are opening up your implementation details to people who should never see it. Let’s say that you have created a Foo class with a Bar method. It’s a really simple interface and sits nicely with the SRP. Internally, perhaps Bar is quite complex and to keep your methods small, you refactor Bar into five or six shorter methods. If these are public, what’s to stop your colleagues using them (or worse, your customers or their other suppliers?). Nothing at all. However, you have locked down your implementation as soon as others begin using it. Maybe, you think, this fits with the OCP? Perhaps not though as when you subclass and start adding new functionality, you might not want the public interface you have saddled yourself with. Now you must implement methods you don’t want or have them throw exceptions and the ISP is nice and broken.
2. You started out making everything public to make it more testable. This may suggest a flaw in your design if it is difficult to test anyway. That aside, to get coverage (if you want 100% – and why not?), you have introduced a heck of a lot more work and a lot more tests. Potentially this is a lot more to rework if things change.
3. When everything is public, you create lots of potential "touch-points". This limits your ability to refactor as the project grows. It also makes it more difficult to understand your class’ interfaces. I am firmly in the camp that says that interfaces should be small.
There are probably another twenty or so reasons why making everyting public is a bad idea, and I am sure that there are many reasons why others may say it is good. I would suggest that some experiments are best left to others with plenty of time on their hands. I also think a lot of this stems from the whole TDD / automated testing thing being relatively new (compared to the history of programming). The tools are not quite what they could be yet and some people prefer to twist the language rather than improve the tool sets. Hammers and screws spring to mind.
Now I’ll duck as the mortars come in 😉
Tony Morris
April 12, 2009 at 9:20 pm (15 years ago)A private method implies you have an uncontrolled, widely-scoped destructive update aka side-effect. Avoid side-effects and therefore, private methods. The TDD advocates have all these crazy names for this type of bad code like "code smells". If your language allows you to avoid side-effects to some extent, do so, but if you are jammed into an impractical language, fight to get out of it or face the possibility that you have to choose between two undesirable options – an untestable private methodd or a testable public method that side-effects uncontrollably (to use mainstream lingo, "breaks encapsulation").
dennis sellinger
April 13, 2009 at 4:26 am (15 years ago)I think we are missing one of the primary cornerstones of object oriented design… encapsulation. I think if you go about making everything public you will definitely reduce the lifetime of your application. Objects are suppose to expose a set of operations which are then accessed by other object by "sending messages". Encapsulation is often the forgotten OO design principal, but I promiss that you ignore it at great risk.
When testing, I think that if you test the public interface you will naturally enough test the private members (or they are un-reachable and can be safely deleted). The only challenge I see is when object construction is private (and this happens often). In this case, exercising the public interface can become hard (but there are of course various techniques for private object creation – and I do exploit them from time to time (even the technique that some might consider iffy).
My point is, reduce the scope of your public interface, then test it completely. Members that are not touched by your tests can then be safely removed.
Radenko Zec
April 13, 2009 at 5:45 am (15 years ago)Hi Dennis.
I must agree with you.Nice thinking.
Thanks for comment
Phil Morrison
April 13, 2009 at 7:47 am (15 years ago)@Tony
from your CV you seem like you should be a bright guy but your statement is pure BS. What side effects tdo you get from private methods? What is your logic for not including them? I see two commenters giving very good reasons for private methods but you give none. You just shout "code smell". I’ll tell you one thing for nothing – if everything you write is public, you are creating a lot of opportunities for unwanted side effects so your code must truly stink.
The public-only debate has no place in object oriented programming.
Emmanuel Caradec
April 13, 2009 at 7:23 pm (15 years ago)Ooops I didn’t indent to create such reactions. Here is what I meant :
class View {
public:
void Redraw() { DrawTriangle(); DrawQuad(); DrawPolygon(): }
private:
void DrawTriangle();
void DrawQuad();
void DrawPolygon();
}
Suppose a imaginary view object with a method Redraw that call internal Draw* methods.
To test the privates, you can call Redraw, read the bitmap and compare with a previous image to see if each one of the shape has been drawn correctly. However if the test fail, you won’t be able to know which method didn’t do its job, or if something screw up horribly (like DrawQuad drawing a triangle ? ) the test may incorrectly pass.
Note that reading the bitmap is not a very good way to test, but hey, all samples are stupids.
One way to test it is to put all your methods public. I was not suggesting this way, but it may be ok, especially if everything hidden behind a abstract interface.
interface IView {
void Redraw();
}
class View : public IView {
public:
void Redraw() { DrawTriangle(); DrawQuad(); DrawPolygon(): }
void DrawTriangle();
void DrawQuad();
void DrawPolygon();
}
Because everything is hidden behind iview, your customer can’t use the privates. In fact its even better since they didn’t even know that it exist.
What I meant was that sometimes (and you have to use your judgment to know when ) many privates methods mean that there is an hidden object that can be refactored out. My sample can be refactored out as :
class Drawer {
void DrawTriangle(View*);
void DrawQuad(View*);
void DrawPolygon(View*);
}
class View {
public:
void Redraw() { DrawTriangle(); DrawQuad(); DrawPolygon(): }
private:
Drawer m_drawer;
}
or
class View : public IView {
public:
void Redraw() { DrawTriangle(); DrawQuad(); DrawPolygon(): }
private:
Triangle m_triangle;
Quad m_quad;
Polygon m_polygon;
}
In either of this way, there is exactly the same public interface on the View object, but you can instantiate drawer (or triangle, quad or polygon ) in a separate test and test them individually.
This is why I said that private methods are a commodity.
*Thats not true of private data and object that are totally required.*
You get extra object and complexity, it is not a solution for everycase.
I figured out tdd alone, reading books and websites, and I may get it completly wrong.
Have fun 🙂