Inspecting Privates

New test-drivers inevitably ask two questions: Is it OK to write tests against private member data? What about private member functions? These are two related but distinct topics that both relate to design choices you must make.

Private Data

The Tell-Don’t-Ask design concept says that you should tell an object to do something and let that object go off and complete its work. If you ask lots of questions of the object, you’re violating Tell-Don’t-Ask. A system consisting of excessive queries to other objects will be entangled and complex. Client C asks an object S for information, does some work that’s possibly a responsibility S could take on, asks another question of S, and so on, creating a tight interaction between C and S. Since S is not taking on the responsibility it should, clients other than C are likely asking similar questions and thus coding duplicate bits of logic to handle the responses.

An object told to do something will sometimes delegate the work to a collaborator object. Accordingly, a test would verify the interaction—“Did the collaborator receive the message?”—using a test double (see Chapter 5, Test Doubles). This is known as interaction-based testing.

Still, not all interactions require spying on collaborators. When test-driving a simple container, for example, you’ll want to verify that the container holds on to any objects added. Simply ask the container what it contains using its public interface and assert against the answer. Tests that verify by inspecting attributes of an object are known as state-based tests.

It’s only fair game to allow clients to know what they themselves stuffed into an object. Add an accessor. (If you’re worried about evil clients using this access for cracking purposes, have the accessor return a copy instead.)

You might have the rare need to hold onto the result of some intermediate calculation—essentially a side effect of making a function call that will be used later. It’s acceptable to create an accessor to expose this value that might not otherwise need to be part of the interface for the class. You might declare the test as a friend of the target class, but don’t do that. Add a brief comment to declare your intent.

 
public​:
 
// exposed for testing purposes; avoid direct production use:
 
unsigned​ ​int​ currentWeighting;

Exposing data solely for the purpose of testing will bother some folks, but it’s more important to know that your code works as intended.

Excessive state-based testing is a design smell, however. Any time you expose data solely to support an assertion, think about how you might verify behavior instead. Refer to Schools of Mock, for further discussion of state vs. interaction testing.

Private Behavior

When test-driving, everything flows from the public interface design of your class. To add detailed behavior, you test-drive it as if your test were a production client of the class. As the details get more detailed and the code more complex, you’ll find yourself naturally refactoring to additional extracted methods. You might find yourself wondering if you should directly write tests against those extracted methods.

The library system defines a HoldingService class that provides the public API for checking out and checking in holdings. Its CheckIn method provides a reasonable (though slightly messy) high-level policy implementation for checking in holdings.

c7/2/library/HoldingService.cpp
 
void​ HoldingService::CheckIn(
 
const​ ​string​& barCode, date date, ​const​ ​string​& branchId)
 
{
 
Branch branch(branchId);
 
mBranchService.Find(branch);
 
 
Holding holding(barCode);
 
FindByBarCode(holding);
 
 
holding.CheckIn(date, branch);
 
mCatalog.Update(holding);
 
 
Patron patronWithBook = FindPatron(holding);
 
 
patronWithBook.ReturnHolding(holding);
 
 
if​ (IsLate(holding, date))
 
ApplyFine(patronWithBook, holding);
 
 
mPatronService.Update(patronWithBook);
 
}

The challenge is the ApplyFine function. The programmer who originally test-drove the implementation of CheckIn started with a simple implementation for a single case but extracted the function as it became complex.

c7/2/library/HoldingService.cpp
 
void​ HoldingService::ApplyFine(Patron& patronWithHolding, Holding& holding)
 
{
 
int​ daysLate = CalculateDaysPastDue(holding);
 
 
ClassificationService service;
 
Book book = service.RetrieveDetails(holding.Classification());
 
 
switch​ (book.Type()) {
 
case​ Book::TYPE_BOOK:
 
patronWithHolding.AddFine(Book::BOOK_DAILY_FINE * daysLate);
 
break​;
 
case​ Book::TYPE_MOVIE:
 
{
 
int​ fine = 100 + Book::MOVIE_DAILY_FINE * daysLate;
 
if​ (fine > 1000)
 
fine = 1000;
 
patronWithHolding.AddFine(fine);
 
}
 
break​;
 
case​ Book::TYPE_NEW_RELEASE:
 
patronWithHolding.AddFine(Book::NEW_RELEASE_DAILY_FINE * daysLate);
 
break​;
 
}
 
}

Each test built around ApplyFine must run in a context that requires a patron first check a book out and then check it in. Wouldn’t it make more sense to exhaust all of the ApplyFine code paths by testing it directly?

Writing tests against ApplyFine directly should make us feel a twinge bad because of the information-hiding violation. More importantly, our design senses should be tingling. ApplyFine violates the SRP for HoldingService: a HoldingService should provide a high-level workflow only for clients. Implementation details for each step should appear elsewhere. Viewed another way, ApplyFine exhibits feature envy—it wants to live in another class, perhaps Patron.

Most of the time, when you feel compelled to test private behavior, instead move the code to another class or to a new class.

ApplyFine isn’t yet a candidate for moving wholesale to Patron, because it does a few things: it asks another function on the service to calculate the number of days past due, determines the book type for the given book, and applies a calculated fine to the patron. The first two responsibilities require access to other HoldingService features, so they need to stay in HoldingService for now. But we can split apart and move the fine calculation.

c7/3/library/HoldingService.cpp
 
void​ HoldingService::ApplyFine(Patron& patronWithHolding, Holding& holding)
 
{
 
unsigned​ ​int​ daysLate = CalculateDaysPastDue(holding);
 
 
ClassificationService service;
 
Book book = service.RetrieveDetails(holding.Classification());
 
patronWithHolding.ApplyFine(book, daysLate);
 
}
c7/3/library/Patron.cpp
 
void​ Patron::ApplyFine(Book& book, ​unsigned​ ​int​ daysLate)
 
{
 
switch​ (book.Type()) {
 
case​ Book::TYPE_BOOK:
 
AddFine(Book::BOOK_DAILY_FINE * daysLate);
 
break​;
 
 
case​ Book::TYPE_MOVIE:
 
{
 
int​ fine = 100 + Book::MOVIE_DAILY_FINE * daysLate;
 
if​ (fine > 1000)
 
fine = 1000;
 
AddFine(fine);
 
}
 
break​;
 
 
case​ Book::TYPE_NEW_RELEASE:
 
AddFine(Book::NEW_RELEASE_DAILY_FINE * daysLate);
 
break​;
 
}
 
}

Now that we look at the moved function in its new home, we see that we still have problems. Asking about the type of book still exhibits possible feature envy. The switch statement represents another code smell. Replacing it with a polymorphic hierarchy would allow us to create more direct, more focused tests. For now, though, we’ve put ApplyFine in a place where we should feel comfortable about making it public so that we can directly test it.

Q.:

Won’t I end up with thousands of extra special-purpose classes by coding this way?

A.:

You will end up with more classes, certainly, but not thousands more. Each class will be significantly smaller, easier to understand/test/maintain, and faster to build! (See Fast Tests, Slow Tests, Filters, and Suites.)

Q.:

I’m not a fan of creating more classes.

A.:

It’s where you start truly taking advantage of OO. As you start creating more single-purpose classes, each containing a small number of single-purpose methods, you’ll start to recognize more opportunities for reuse. It’s impossible to reuse large, highly detailed classes. In contrast, SRP-compliant classes begin to give you the hope of reducing your overall amount of code.

What if you’re dealing with legacy code? Suppose you want to begin refactoring a large class with dozens of member functions, many private. Like private behavior, simply relax access and add a few tests around some of the private functions. (Again, don’t worry about abuse; it doesn’t happen.) Seek to move the function to a better home later. See Chapter 8, Legacy Challenges for more on getting legacy code under control.

..................Content has been hidden....................

You can't read the all page of ebook, please click here login for view all page.
Reset
3.138.122.11