2

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?

Community
  • 1
  • 1
mindas
  • 26,463
  • 15
  • 97
  • 154
  • 1
    You've basically brought up a topic for discussion rather than asked a single concrete question that can be answered. At least for the part where you're asking about a better way to write the particular code, you might be better off asking on http://codereview.stackexchange.com. – Jerry Coffin Feb 17 '11 at 16:55
  • If you think this is more of a discussion topic than a question, feel free to convert it to community wiki. As for codereview.. it only has 229 questions in the entire site and the attendance is abysmal. I didn't know people are not allowed to post code on SO questions... Moreover, this is more of a pseudo-code rather than a concrete code. – mindas Feb 17 '11 at 16:59
  • 2
    Not sure why you would be will to go much effort to avoid 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. – Peter Lawrey Feb 17 '11 at 17:17
  • @Peter, I think you should re-qualify your comment into an answer; it makes a lot of sense. But the consequence of what you said is that implementing unit testing graciously often _requires_ reflection. This is quite an important axiom. – mindas Feb 17 '11 at 17:22

6 Answers6

4

My go-to answer for this type of thing is a "test proxy". In your test package, derive a subclass from your system under test, which contains "pass-through" accessors to protected data.

Advantages:

  • You can directly test, or mock, methods you don't want made public.
  • Since the test proxy lives in the test package, you can ensure it is never used in production code.
  • A test proxy requires far fewer changes to code in order to make it testable than if you were testing the class directly.

Disadvantages:

  • The class must be inheritable (no final)
  • Any hidden members you need to access cannot be private; protected is the best you can do.
  • This isn't strictly TDD; TDD would lend itself to patterns that didn't require a test proxy in the first place.
  • This isn't strictly even unit testing, because at some level you are dependent on the "integration" between the proxy and the actual SUT.

In short, this should normally be a rarity. I tend to use it only for UI elements, where best practice (and default behavior of many IDEs) is to declare nested UI controls as inaccessible from outside the class. Definitely a good idea, so you can control how callers get data from the UI, but that also makes it difficult to give the control some known values to test that logic.

KeithS
  • 70,210
  • 21
  • 112
  • 164
3

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

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • My initial reason for writing this question was an argument between myself and a colleague who argued that making inner class outer is preferable over reflection (just for testability sake). So far I haven't seen any arguments which would justify the former. Other solutions (such as suggested by KeithS) have other disadvantages and I still think that reflection (especially when wrapped deep inside unit test libraries) does more good than bad. Finally, it also became clear that in certain cases (like this one) price needs to be paid and something has to give. – mindas Feb 18 '11 at 10:34
  • What you really need for Java to have support for unit tests which can ignore private etc modifiers. – Peter Lawrey Feb 18 '11 at 11:19
2

I don't see how information hiding, in the abstract, is reducing your testability.

If you were injecting the SomethingThatExpectsMyInterface used in this method rather than constructing it directly:

public void someMethod() {
   // calculate x, y and z
   SomethingThatExpectsMyInterface something = ...;
   something.submit(new InnerFoo(x, y, z));
}

Then in a unit test you could inject this class with a mock version of SomethingThatExpectsMyInterface and easily assert what happens when you call someMethod() with different inputs - that the mockSomething receives arguments of certain values.

I think you may have over-simplified this example anyway as InnerFoo cannot be a private class if SomethingThatExpectsMyInterface receives arguments of its type.

"Information Hiding" doesn't necessarily mean that the objects you pass between your classes need to be a secret - just that you aren't requiring external code using this class to be aware of the details of InnerFoo or the other details of how this class communicates with others.

Carl Manaster
  • 39,912
  • 17
  • 102
  • 155
matt b
  • 138,234
  • 66
  • 282
  • 345
  • "InnerFoo cannot be a private class if SomethingThatExpectsMyInterface receives arguments of it's type" -- SomethingThatExpectsMyInterface expects instances of MyInterface, and InnerFoo implements that. – mindas Feb 17 '11 at 17:07
  • Ok, that doesn't change my main point about testability though - if you injected the instance of `SomethingThatExpectsMyInterface` into this class, then you could easily test what type of objects your class is sending to that interface with a mock/test double. Information hiding and testability are not opposed; however, tight coupling and testability are. http://misko.hevery.com/2008/07/30/top-10-things-which-make-your-code-hard-to-test/ – matt b Feb 17 '11 at 17:12
  • One of the implicit points I was trying to make was that TDD cannot be achieved without reflection _and_ without any extra cost when it comes to accessing private data. This is not necessarily bad, but I wanted to get this confirmed. Speaking of injection -- I don't know how can it be achieved without reflection, at least in Java. – mindas Feb 17 '11 at 17:27
  • @mindas instead of your class constructing a `SomethingThatExpectsMyInterface` instance on it's own, it *receives* one either through the constructor or a setter. This way the code that uses your class "injects" the dependencies (`SomethingThatExpectsMyInterface`) themselves. – matt b Feb 17 '11 at 21:38
  • Also I'm curious - what value do you think keeping the `InnerFoo` class private has? – matt b Feb 17 '11 at 21:42
  • See http://aarontgrogg.com/wp-content/uploads/2009/09/How-to-Build-API-and-why-it-matters.pdf (there's also a talk on YouTube). One of my favourite talks. One of the slides has title "Minimize Accessibility of Everything". – mindas Feb 18 '11 at 10:24
2

SomethingThatExpectsMyInterface can be tested outside Foo, right? You can call its submit() method with your own test class that implements MyInterface. So that unit is taken care of. Now you are testing Foo.someMethod() with that well-tested unit and your untested inner class. That's not ideal - but it's not too bad. As you test-drive someMethod(), you are implicitly test-driving the inner class. I know that's not pure TDD, by some strict standards, but I would consider it sufficient. You're not writing a line of the inner class except to satisfy a failing test; that there's a single level of indirection between your test and the tested code doesn't constitute a big problem.

Carl Manaster
  • 39,912
  • 17
  • 102
  • 155
  • The biggest question here is about testing InnerFoo to get 100% coverage. Average TDD pundit could argue that InnerFoo could have been made much more testable if extracted, and this is the source of conflict. – mindas Feb 17 '11 at 17:41
  • 2
    If InnerFoo is as simple as in the example, then i don't need to see why it needs to be tested separately to Foo, any more than any other dozen lines of code with braces round it would be. You wouldn't argue for extracting a for-loop in the middle of a method into something externally testable, would you? – Tom Anderson Feb 17 '11 at 18:36
0

In your example it looks like the Foo class really needs a collaborator InnerFoo.

In my opinion the tension between information hiding and testability is solved by the "composite simpler than the sum of its parts" motto.

Foo is a facade over a composite (just InnerFoo in your case, but does not matter.) The facade object should be tested on its intended behaviour. If you feel that the InnerFoo object code is not driven enough by the tests on the behaviour of Foo, you should consider what InnerFoo represents in your design. It may be that you miss a design concept. When you find it, name it, and define its responsibilities, you may test its behaviour separately.

xpmatteo
  • 11,156
  • 3
  • 26
  • 25
-1

what I don't like is making any sacrifices when it comes to information hiding

First, work with Python for a few years.

private is not particularly helpful. It makes extension of the class hard and it makes testing hard.

Consider rethinking your position on "hiding".

S.Lott
  • 384,516
  • 81
  • 508
  • 779