views:

504

answers:

5

Is it better to add functions that return the internal state of an object for unit testing, as opposed to making the testing class a friend? - especially, when there is no use for the functions except for the case of unit testing.

+2  A: 

I recommend using accessors rather than allowing access via public members or friend classes.

I don't think that using friend class actually gets you any benefits, and it has the potential to make your life a lot worse down the road. If your code is going to stay around for a long time there's a good chance that it might be used in ways that you don't anticipate. The access functions might be only used for testing now, but who knows what will happen in the future? Using accessors rather than providing direct access to variables gives you a lot more flexibility, and it has a very low cost.

The other argument is that using accessors rather than public members is a good habit. Developing good habits is an important skill as a programmer.

James Thompson
+1 for habit development.
Hooked
To make sure I understand you, an accessor for the variable double time;I create would create:double time() const;
bias
Yes, for two reasons: 1, it tells other programmers what you want them to be able to do with the data, and 2: the compiler can often optimize const functions that it could not without the const identifier.
Hooked
Oh, yes, note that having a variable time and a function time() won't compile; most people have the actual variable with a trailing underscore like time_, and the accessor like time() { return time_; }
Hooked
+6  A: 

Unit tests should 95% of the time only test the publicly exposed surface of a class. If you're testing something under the covers, that's testing implementation details, which is inherently fragile, because you should be able to easily change implementation and still have the tests work. Not only is it fragile, but you could also be tempted into testing things that aren't actually possible in planned usage scenarios, which is a waste of time.

If the point of the accessors you want to add is just to test whether the function had the desired effect, your class design may violate another principle, which is that a state-machine-like class should always make it clear what state its in, if that affects what happens when people interact with the class. In that case, it'd be right to provide those read-only accessors. If it doesn't affect the class's behavior, refer back to my previous bit about implementation details.

And as you rightly said, cluttering up the public surface of a class with unused stuff is also undesirable for its own reasons.

If I had to pick between accessors and friending in your case, I would choose friending, simply because you own your test and can change it in a pinch. You may not own the code by the clown who finds a way to use your extra accessors, and then you'll be stuck.

Drew Hoskins
+3  A: 

I'll disagree with the accepted answer and instead recommend the use of a friend class.

Part of the state you are testing is probably specific to the implementation of your class; you're testing details that other code normally doesn't know about or care about and shouldn't rely on. Public accessor functions make these implementation details part of the class's interface. If the internal state you are testing is not part of the intended interface, it shouldn't be visible through public functions. From a purist point of view you're stuck between two wrong answers, as friend classes are also technically part of the public interface. In my mind the question becomes, which option is less likely to lead to poor coding choices down the road? Going with a set of implementation-dependent public accessor functions will inadvertantly encourage an implementation-dependent conceptual model of the the class, leading to implementation-dependent use of the class. A single friend class, appropriately named and documented, is less likely to be abused.

While in general I strongly agree with the recommendation to prefer accessor functions over direct access to member variables, I don't agree that this best practice applies to unit testing of implementation-dependent internal state. A reasonable middle ground is to use private accessor functions to those pieces of state your unit test will care about, and be disciplined enough to use the accessor functions in your unit tests. Just my opinion.

Darryl
+1  A: 

How about making internal state "protected"? And then do unittest using derived class.

rein
+1  A: 

I think there needs to be a distinction between future-proofing the class by providing accessors to its users if it makes sense to do so, and improving testability. I'm also not a big fan of befriending classes for the sole purpose of testing as this introduces a tight coupling in a place where I prefer not to have it.

If the sole use of the accessors is to provide a way for the test cases to check internal state of the class, it normally doesn't make sense to expose them publicly. It also can tie down implementation details that you might wish to change later on but then find you can't because someone else is using said accessors.

My preferred solution for this would be to provide protected accessor functions to communicate clearly to the class's users that these are not part of the public interface. Your tests would then create a minimal derived class of the original which contains call-through stubs for the parent's functions but also makes the accessors public so you can make use of them in the test cases.

Timo Geusch