Is refactoring for the purpose of unit testing a code smell?

Question

Let me preface this question by saying that I get the need for unit testing. I painfully get it. You have to do it to ensure that future modifications don't adversely affect the application in ways you did not expect. It's also good practice to utilize a test driven methodology to ensure your current modifications work as expected while providing a solid testing base for adding edge cases (bugs) later. I also understand that testing is a requirement for any company that will, or plans to be, subject to an IT audit. And finally, I get that if your code is testable, it is probably written using solid design principles.

The quandary that I have is that coding to an interface for the purpose of supporting unit testing alone, seems like a contrived complexity.

Take the case where you have a database application that does not have the requirement to be database agnostic. You've already coded a DAL that communicates with that database and BL that uses that DAL for persistence. Integration tests are in place with the current architecture that utilize a development database for test data.

To support 100% code coverage on unit tests you would now need to introduce interfaces for both the DAL and the BL, have concrete implementations of those interfaces, and use dependency or constructor injection to utilize those interfaces based on the context of the runtime.

Does that not introduce a contrived complexity? A side question I guess is where is the line drawn between unit and integration testing in these cases? It's certainly feasible that the data model can change and you'd want integration testing in addition to unit testing because it's clear that unit testing will succeed where integration testing can fail in that case.

Answer 1

1. 100% code coverage is an impractical goal - let's face facts here, to get that coverage, quite a few tests are going to be the sort to cover the code but not test it. Worse, you're spending a lot of time to make tests that are unlikely to cause error, or have minimal impact if errors occur.

2. All database applications require database agnostic behavior - oh if I had a dollar for every time some said "oh, that isn't going to change". Sure, your database might not change, but what about licensing that code to 3rd parties? What about putting it on a mobile platform? What about putting it behind a web service? What about all the weird and interesting things we'll do in 5-10 years that you can't envision?

3. (IMO) Don't change your design for testability alone - and now to the meat of your question and the more contentious point I will make: You are absolutely right that testable code is probably well designed code. If you are going to refactor your code, don't do it to make the code more testable, make it to be better designed. Unit tests serve as a great proxy for alternative needs. If it's hard to test, it will likely be hard to reuse. But sometimes you make a trade off that things will be hard to use in exchange for performance, or a nice interface, or to simplify code... Don't make your production code worse in favor of your test code. The tests are there to serve you after all.

But do be cautious - the common case is that hard to test code means hard to reuse code, and well done refactoring will help your test code and your production code.

Answer 2

Some good answers here, but I would like to add something to answer this part of your question:

To support 100% code coverage on unit tests you would now need to introduce interfaces for both the DAL and the BL, have concrete implementations of those interfaces, and use dependency or constructor injection to utilize those interfaces based on the context of the runtime.

This depends on the way you have designed your DAL and your BL. A well designed DAL should allow objects could be created completely without a database, in-memory. And your BL should should not rely on getting its DAL objects from a database - the BL should use some approach like the repository pattern to retrieve any DAL objects. This will allow easy automatic testing for the DAL and the BL without adding interfaces for each and every DAL object - you will only need interfaces for your repositories (and repository mocks for your tests).

You could argue that this way you don't produce real unit tests this way for your BL, because you cannot test them in isolation from your DAL objects, but as long as your DAL objects are not much more than data containers, this is IMHO perfectly acceptable and a pragmatic compromise.

Does that not introduce a contrived complexity?

Your system will contain more pieces, so beeing more complex, but each piece is becoming less complicated - which will increase the overall evolvability of your system.

Answer 3

You should have unit tests for both your DAL and BL code. The both portions of your code should work independently of each other and not depend on implementation details of the other. Writing Unit Tests not only tests your code, but also is a tool to ensure that there is that separation of concerns.

It may seems like a large amount of work and overhead, but imagine 6 months down the road you database just isn't scaling like you hoped. You decide to switch to a key value document store like reddis. As long as your Reddis implementation fully implements the DAL interface and it passes all your unit tests you wrote for it, you can be extremely confident that switching out those classes should be without a hitch (ignoring data migration issues of course).

While it seems hard now, once you get the hang of writing code for unit tests, you will find it less of a chore and more of a tool for writing better code. You will find your integration tests tend to be brittle and you will write them less and less.

I would argue that needing to refactor your code to support unit tests is a code smell, which is why we write them.

Answer 4

Yes, there is a catch-22 involved in some unit testing: to be able to test the code you have to sometimes change it. This is sometimes unavoidable.

As a rule of thumb, once you've crossed over into Data Access, it's not really a unit test any more. Think of a unit test as something that purely exercises your logic and internal data transformations, without doing any IO.

Which brings us back to the need for faking data access with interfaces and fake implementations.