Fixing Unclean Code
What? We wrote one line of production code and three lines of test code and we have a problem? Indeed. It’s extremely easy to introduce deficient code even in a small number of lines. TDD provides the wonderful opportunity to fix such small problems as they arise, before they add up to countless small problems (or even a few big problems).
We read both the test and production code we’ve written, looking for deficiencies. We decide that the assertion in our test isn’t reader-friendly.
| | ASSERT_THAT(encoded, testing::Eq("A")); |
Much as the test declaration (the combination of test case and test name) should read like a sentence, we want our asserts to do the same. We introduce a using directive to help.
| c2/7/SoundexTest.cpp | |
| | #include "gmock/gmock.h" |
* | using ::testing::Eq; |
| | |
| | TEST(SoundexEncoding, RetainsSoleLetterOfOneLetterWord) { |
| | Soundex soundex; |
| | auto encoded = soundex.encode("A"); |
* | ASSERT_THAT(encoded, Eq("A")); |
| | } |
Now we can paraphrase the assertion with no hiccups: assert that the encoded value is equal to the string "A".
Our small change is a refactoring, a code transformation in which we retain existing behavior (as demonstrated by the test) but improve the design. In this case, we improved the test’s design by enhancing its expressiveness. The namespace of Eq is an implementation detail not relevant to the test’s meaning. Hiding that detail improves the level of abstraction in the test.
Code duplication is another common challenge we face. The costs and risks of maintenance increase with the amount of code duplication.
Our Soundex class contains no obvious duplication. But looking at both the test and production code in conjunction reveals a common magic literal, the string "A". We want to eliminate this duplication. Another problem is that the test name (RetainsSoleLetterOfOneLetterWord) declares a general behavior, but the implementation supports only a specific, single letter. We want to eliminate the hard-coded "A" in a way that solves both problems.
How about simply returning the word passed in?
| c2/8/SoundexTest.cpp | |
| | class Soundex |
| | { |
| | public: |
| | std::string encode(const std::string& word) const { |
* | return word; |
| | } |
| | }; |
At any given point, your complete set of tests declares the behaviors you intend your system to have. That implies the converse: if no test describes a behavior, it either doesn’t exist or isn’t intended (or the tests do a poor job of describing behavior).
Where am I going with this? We have one test. It says we support one-letter words. Therefore, we can assume that the Soundex code needs to support only one-letter words—for now. And if all words are one letter, the simplest generalized solution for our test is to simply return the whole word passed to encode.
(There are other TDD schools of thought about what we might have coded at this point. One alternate technique is triangulation[6]—see Triangulation—where you write a second, similar assertion but with a different data expectation in order to drive in the generalized solution. You’ll discover more alternate approaches throughout the book, but we’ll keep things simple for now.)
Our changes here are small, bordering on trivial, but now is the time to make them. TDD’s refactoring step gives us an opportunity to focus on all issues, significant or minor, that arise from a small, isolated code change. As we drive through TDD cycles, we’ll use the refactoring step as our opportunity to review the design impacts we just made to the system, fixing any problems we just created.
Our primary refactoring focus will be on increasing expressiveness and eliminating duplication, two concerns that will give us the most benefit when it comes to creating maintainable code. But we’ll use other nuggets of design wisdom as we proceed, such as SOLID class design principles and code smells.