Many times I am involved in the design/implementation of APIs I am facing this dilemma.
I am a very strong supporter of information hiding and try to use various techniques for that, including but not limited to inner classes, private methods, package-private qualifiers, etc.
The problem with these techniques is that they tend to prevent good testability. And while some of these techniques can be resolved (e.g. package-privateness by putting a class into the same package), others are not so easy to tackle and either requires reflection magic or other tricks.
Let's look at concrete example:
public class Foo {
SomeType attr1;
SomeType attr2;
SomeType attr3;
public void someMethod() {
// calculate x, y and z
SomethingThatExpectsMyInterface something = ...;
something.submit(new InnerFoo(x, y, z));
}
private class InnerFoo implements MyInterface {
private final SomeType arg1;
private final SomeType arg2;
private final SomeType arg3;
InnerFoo(SomeType arg1, SomeType arg2, SomeType arg3) {
this.arg1 = arg1;
this.arg2 = arg2;
this.arg3 = arg3;
}
@Override
private void methodOfMyInterface() {
//has access to attr1, attr2, attr3, arg1, arg2, arg3
}
}
}
There are strong reasons not to expose InnerFoo
- no other class, library should have access to it as it does not define any public contract and the author deliberately didn't want it to be accessible. However to make it 100% TDD-kosher and accessible without any reflection tricks, InnerFoo
should be refactored like this:
private class OuterFoo implements MyInterface {
private final SomeType arg1;
private final SomeType arg2;
private final SomeType arg3;
private final SomeType attr1;
private final SomeType attr2;
private final SomeType attr3;
OuterFoo(SomeType arg1, SomeType arg2, SomeType arg3, SomeType attr1, SomeType attr2, SomeType attr3) {
this.arg1 = arg1;
this.arg2 = arg2;
this.arg3 = arg3;
this.attr1 = attr1;
this.attr2 = attr2;
this.attr3 = attr3;
}
@Override
private void methodOfMyInterface() {
//can be unit tested without reflection magic
}
}
This examply only involves 3 attrs, but it is pretty reasonable to have 5-6 and the OuterFoo
constructor would then have to accept 8-10 parameters! Add getters on top, and you already have 100 lines of completely useless code (getters would be also required to get these attrs for testing). Yes, I could make the situation a bit better by providing a builder pattern but I think this is not only over-engineering but also defeats the purpose of TDD itself!
Another solution for this problem would be to expose a protected method for class Foo
, extend it in FooTest
and get the required data. Again, I think this is also a bad approach because protected
method does define a contract and by exposing it I have now implicitly signed it.
Don't get me wrong. I like to write testable code. I love concise, clean APIs, short code blocks, readability, etc. But what I don't like is making any sacrifices when it comes to information hiding just because it is easier to unit test.
Can anybody provide any thoughts on this (in general, and in particular)? Are there any other, better solutions for given example?
I think you should reconsider using reflection.
It has its own downsides but if it allows you to maintain the security model you want without dummy code, that may be a good thing. Reflection is often not required, but sometimes there is no good substitute.
Another approach to information hiding is to treat the class/object as a black box and not access any non-public methods (Though this can allow tests to pass for the "wrong" reasons i.e. the answer is right but for the wrong reasons.)