We’ve all heard that you should use tests to drive your design but what does that mean, exactly? I’ve run across some interesting design decisions that were made under the flag of testability though I think they missed the mark insofar as driving the design. These were more a degradation of design.

Making Methods Public

Methods are promoted from private or protected to public so they can be tested directly. This usually arises from private methods that are extracted but grow over time to contain increasingly complex logic. After a while it is no longer convenient to test them through the public API of the class. It would be nice to test them directly.

The problem with this is that it muddies the public API of your class. If a method belongs to the internals of this class it should remain private. Making methods public that shouldn’t be public makes your API confusing to your consumers.

This hints that the class is trying to do too many things; a Single Responsibility violation. There is another class or two trying to get out. If there’s a hairy bit of logic you would like to test separately but is hidden behind a private method, pull it out into a new class where it can be a public concern of that class.

Making Members Protected

Members are promoted from private to protected so they can be overridden. This one tripped me up. I was tagged in a code review where some private members were changed to protected. There were no descendants that needed access to the members nor was there intention of extending the class. So why was this change made? I asked and the reason was so that you could “test” this class by extending it and then overriding these members.

This increases coupling and breaks encapsulation in the name of testing. But the big problem is you aren’t truly testing the class at this point. By extending the class you are testing that subclass and only incidentally testing the parent class. It might be close enough for government work but not for software development. Another possibility is promoting access in languages that allow package-level access, thus granting access to tests that are in the same package or namespace. I could buy that there would be circumstances where this is necessary but would save it for a last resort.

This indicates a possible Dependency Inversion violation. Instead of making these members protected, pass their values in at object construction time. You now have the added benefit of a more flexible design.

Singleton Modifier

I ran into some classes that used a singleton to create a database connection. This created an obvious testability problem so I asked around about it, figuring it was a legacy approach that had since been replaced by something that could be injected. Turns out this was the general way to retrieve a database connection and the testability problem had been addressed in an interesting way: This singleton came with a `setInstance` method allowing one to mock or otherwise subclass and set that instance.

Singletons are already problematic because they are (socially acceptable) global state. The only thing worse than using global state is reaching out from deep within an implementor and modifying it. This is sure to come back and haunt you. Just having that `setInstance` hanging off there will be enough temptation for someone to abuse it outside of the tests.

This is a Dependency Inversion and encapsulation violation. Get rid of the singleton. Pass your collaborators into the class and relieve it from having to construct it’s own dependencies.

In Conclusion

In all of these cases design was modified to accommodate tests. Which is good, right? We want tests to drive the design, correct? Well, sort of. Pain encountered while testing means that you have a deficit in your design. Instead of bending over backwards to accommodate your testing, use this “testing pain” as a sign to take a step back and look at the overall design. If you’re having trouble coming up with anything pull in a colleague or two. Sometimes all you need is a fresh perspective. In general, a good place to start is to look for basic violations such as SOLID, Demeter, encapsulation, and separation of concerns. That will cover most of your bases.

Advertisements