Is a class that is hard to unit test badly designed?

Question

I am now doing unit testing on an application which was written over the year, before I started to do unit-testing diligently. I realized that the classes I wrote are hard to unit test, for the following reasons:

1. Relies on loading data from database. Which means I have to setup a row in the table just to run the unit test (and I am not testing database capabilities).
2. Requires a lot of other external classes just to get the class I am testing to its initial state.

On the whole, there doesn't seem to be anything wrong with the design except that it is too tightly coupled (which by itself is a bad thing). I figure that if I have written automated test cases with each of the class, hence ensuring that I don't heap extra dependencies or coupling for the class to work, the class might be better designed.

Does this reason holds water? What are your experiences?

Answer 1

Yes you are right. A class which is not unit testable hard to unit test is (almost always) not well designed (there are exceptions, as always, but these are rare - IMHO one should better not try to explain the problem away this way). Lack of unit tests means that it is harder to maintain - you have no way of knowing whether you have broken existing functionality whenever you modify anything in it.

Moreover, if it is (co)dependent with the rest of the program, any changes in it may break things even in seemingly unrelated, far away parts of the code.

TDD is not simply a way to test your code - it is also a different way of design. Effectively using - and thinking about using - your own classes and interfaces from the very first moment may result in a very different design than the traditional way of "code and pray". One concrete result is that typically most of your critical code is insulated from the boundaries of your system, i.e. there are wrappers/adapters in place to hide e.g. the concrete DB from the rest of the system, and the "interesting" (i.e. testable) code is not within these wrappers - these are as simple as possible - but in the rest of the system.

Now, if you have a bunch of code without unit tests and want to cover it, you have a challenge. Mocking frameworks may help a lot, but still it is a pain in the ass to write unit tests for such a code. A good source of techniques to deal with such issues (commonly known as legacy code) is Working Effectively with Legacy Code, by Michael Feathers.

Answer 2

Yes, the design could be better with looser coupling, but ultimately you need data to test against.

You should look into Mocking frameworks to simulate the database and the other classes that this class relies on so you can have better control over the testing.

Answer 3

While there isn't a way to establish if a class is "well designed" or not, at least the first thing you mention is usually a sign of a questionable design.

Instead of relying on the database, the class you are testing should have a dependency on an object whose only responsibility is getting that data, maybe using a pattern like Repository or DAO.

As for the second reason, it doesn't necessarily highlight a bad design of the class; it can be a problem with the design of the tests (not having a fixture supertype or helpers where you can setup the dependencies used in several tests) and/or the overall architecture (i.e. not using factories or inversion of control to inject the corresponding dependencies)

Also, you probably shouldn't be using "real" objects for your dependencies, but test doubles. This helps you make sure you are testing the behavior of that one class, and not that of its dependencies. I suggest you look into mocking frameworks.

Answer 4

Ideally, the large majority of your classes will be unit-testable, but you'll never get to 100% since you're bound to have at least one bit that is dedicated to binding the other classes together to an overall program. (It's best if that can be exactly one place that is obviously and trivially correct, but not all code is as pleasant to work with.)

Answer 5

I've found that dependency injection is the design pattern that most helps make my code testable (and, often, also reusable and adaptable to contexts that are different from the original one that I had designed it for). My Java-using colleagues adore Guice; I mostly program in Python, so I generally do my dependency injection "manually", since duck typing makes it easy; but it's the right fundamental DP for either static or dynamic languages (don't let me get started on "monkey patching"... let's just say it's not my favorite;-).

Once your class is ready to accept its dependencies "from the outside", instead of having them hard-coded, you can of course use fake or mock versions of the dependencies to make testing easier and faster to run -- but this also opens up other possibilities. For example, if the state of the class as currently designed is complex and hard to set up, consider the State design pattern: you can refactor the design so that the state lives in a separate dependency (which you can set up and inject as desired) and the class itself is mostly responsible for behavior (updating the state).

Of course, by refactoring in this way, you'll be introducing more and more interfaces (abstract classes, if you're using C++) -- but that's perfectly all right: it's a excellent principle to "program to an interface, not an implementation".

So, to address your question directly, you're right: the difficulty in testing is definitely the design equivalent of what extreme programming calls a "code smell". On the plus side, though, there's a pretty clear path to refactor this problem away -- you don't have to have a perfect design to start with (fortunately!-), but can enhance it as you go. I'd recommend the book Refactoring to Patterns as good guidance to this purpose.

Answer 6

For me, code should be designed for testability. The other way around, I consider non-testable or hard to test code as badly designed (regardless of its beauty).

In your case, maybe you can mock external dependencies to run real unit tests (in isolation).

Answer 7

I'll take a different tack: the code just isn't designed for testability, but that does not mean its necessarily badly designed. A design is the product of competing *ilities, of which testability is only one of them. Every coding decision increases some of the *itilies while decreasing others. For example, designing for testability generally harms its simplicity/readability/understandability (because it adds complexiety). A good design favors the most important *ilities of your situation.

Your code isn't bad, it just maximizes the other *ilities other than testability. :-)

Update: Let me add this before I get accused of saying designing for *testability isn't important

The trick of course is to design and code to maximize the good *ilities, particularly the important ones. Which ones are the important ones depends on your situation. In my experience in my situations, designing for testability has been one of the more important *ilities.

Answer 8

I might have an ideal solution for you to consider... the Private Accessor

Recently I served in a role where we used them prolifically to avoid the very situation you're describing -- reliance upon artificial data maintained within the primary data-store.

While not the simplest technology to implement, after doing so you'll be able to easily hard-define private members in the method being tested to whatever conditions you feel they should possess, and right from the unit-test code (so no loading from the database). Also you'll have accomplished the goal without violating class protection levels.

Then it's basic assert & verification for desired conditions as normal.