Doing What It Takes to Clarify Tests
For our next test, we tackle the case where two adjacent letters encode to the same digit. Per Soundex rule #3, such duplicate letters get encoded as a single digit. The rule also states that it applies to the first letter. Let’s deal with the first case now and worry about the first letter next.
| c2/29/SoundexTest.cpp | |
| | TEST_F(SoundexEncoding, CombinesDuplicateEncodings) { |
| | ASSERT_THAT(soundex.encode("Abfcgdt"), Eq("A123")); |
| | } |
That’s a confusing test! To understand why Abfcgdt encodes to A123, we have to know that b and f both encode to 1, c and g both encode to 2, and d and t both encode to 3. We can learn these facts from reading other tests, such as ReplacesConsonantsWithAppropriateDigits, but maybe we should make the test more direct.
Let’s add a series of precondition assertions to help readers make the connection.
| c2/30/SoundexTest.cpp | |
| | TEST_F(SoundexEncoding, CombinesDuplicateEncodings) { |
| | |
* | ASSERT_THAT(soundex.encodedDigit('b'), Eq(soundex.encodedDigit('f'))); |
* | ASSERT_THAT(soundex.encodedDigit('c'), Eq(soundex.encodedDigit('g'))); |
* | ASSERT_THAT(soundex.encodedDigit('d'), Eq(soundex.encodedDigit('t'))); |
| | |
| | ASSERT_THAT(soundex.encode("Abfcgdt"), Eq("A123")); |
| | } |
The assertion doesn’t compile, since encodedDigit is private. We choose to simply make encodedDigit public.
| c2/30/Soundex.h | |
* | public: |
| | std::string encodedDigit(char letter) const { |
| | // ... |
| | } |
| | |
* | private: |
| | // ... |
Uh-oh...I’m sensing consternation.
- Q.:
-
Wait, no! You can’t just go making private functions public.
- A.:
-
We do have other solutions. We could make the test code a friend of the Soundex class, but a friend is usually a poor choice, and that’s no different when test-driving. We could move the function to another class, possibly named SoundexDigitEncoder, but that seems overkill. We could also forego the preconditions and find another way to make our test easier to read.
- Q.:
-
We’ve always been taught to not expose private implementation details. Shouldn’t you follow that time-honored rule?
- A.:
-
First, we don’t willy-nilly expose everything, just things we need. Second, we’re not exposing implementation details so much as broadening the public interface of the Soundex class. Yes, we’re adding to it a function that production clients of Soundex shouldn’t need, but our test—a client—needs it. We take on a low risk of potential abuse and in return guarantee less wasted time for developers who must read our test in the future.
We could make the case for precondition assertions in some of the other tests we’ve written for Soundex. Use them sparingly, though. Often, introducing a meaningfully named constant or local variable can be a simpler, as-effective solution. Also, the compulsion to add a precondition assertion might suggest you are missing another test. Are you? Go add it and then see whether having that test eliminates the need for the precondition assertion.
To make CombinesDuplicateEncodings pass, we could introduce a local variable that would track the last digit appended, updating it each iteration of the loop. But that seems muddy. Let’s start with a declaration of intent.
| c2/31/Soundex.h | |
| | std::string encodedDigits(const std::string& word) const { |
| | std::string encoding; |
| | for (auto letter: word) { |
| | if (isComplete(encoding)) break; |
* | if (encodedDigit(letter) != lastDigit(encoding)) |
| | encoding += encodedDigit(letter); |
| | } |
| | return encoding; |
| | } |
We know what lastDigit needs to do. We think for a moment and come up with one way that provides the how.
| c2/31/Soundex.h | |
| | std::string lastDigit(const std::string& encoding) const { |
| | if (encoding.empty()) return ""; |
| | return std::string(1, encoding.back()); |
| | } |