Simple Design

If you knew nothing whatsoever about design, you might consider following three simple rules when practicing TDD.

• Ensure your code is always highly readable and expressive.

• Eliminate all duplication, as long as doing so doesn’t interfere with the first rule.

• Ensure that you don’t introduce unnecessary complexity into your system. Avoid speculative constructs (“I just know we’re gonna have to support many-to-many someday”) and abstractions that don’t add to your system’s expressiveness.

The final rule is important, but the other two hold slightly higher priority. In other words, prefer introducing an abstraction such as a new member function or class if it improves expressiveness.

Following these rules can give you an eminently maintainable system.

The Cost of Duplication

In our coding exercises, we’ve focused a lot on eliminating duplication. But why?

Duplication is perhaps the biggest cost in maintaining a codebase over time. Imagine your system requires two lines of code to audit important events. Sure, they could go into a helper member function, but it’s just two lines of code, right? When you need to add an audit event, it’s simple enough to find another audit point and then copy and paste the two lines.

Two lines duplicated doesn’t sound bad, but now visualize those two lines of audit code repeated one hundred times throughout your codebase. And then imagine that the auditing requirements change ever so slightly and you need to add a third line of logic. Uh-oh. You’ll have to find all one hundred change points, add the missing line, and retest everything, all at considerably higher cost than if there were a single place to do so. What if one of the duplicate pair of lines is slightly different? You may have to spend additional analysis time to determine whether the variation is deliberate. What if you find only ninety-nine of the duplicates but neglect to find and fix the hundredth? You’ve shipped defective code.

Most developers are lazy about creating new member functions, and sometimes they resist doing so because of dubious claims about degrading performance. But ultimately they are only creating more future work for themselves.

That’s not the only way duplicate code emerges. Imagine you’ve been told to add a variant to an existing feature. Your changes require a half-dozen lines of code executed if some condition exists, else execute a half-dozen lines of existing code.

You discover that the existing feature is implemented in an untested 200-line member function. The right thing would be a design that extracts the 195 or so lines of commonality and allows you to introduce your variant as an extension. Perhaps the template method or strategy design pattern would provide the basis for an appropriate solution.

Most programmers don’t do the right thing, not because they don’t know how but because they are lazy and fearful. “If I change the existing code and it breaks, I’ll get blamed for breaking something that I shouldn’t have been messing with in the first place.” It’s easier to copy the 200 lines, make the changes needed, and move on.

Because of this somewhat natural tendency toward duplication, most large systems contain substantially more code than required. The extra code dramatically increases maintenance costs and risks.

You have the potential to stave off this systematic degradation by incrementally refactoring as part of the TDD cycle.

The Portfolio Manager

Let’s take a look at how Beck’s notion of simple design plays out in developing a small subsystem.

In test-driving the portfolio manager, we’ve ended up with the following effort. (Note: Much of the code for this example will be built behind the scenes. I’ll present only the code that’s relevant to our design discussions. Refer to the source code download for full listings.)

c6/1/PortfolioTest.cpp
​	#include "gmock/gmock.h"
​	#include "Portfolio.h"
​
​	​using​ ​namespace​ ::testing;
​
​	​class​ APortfolio: ​public​ Test {
​	​public​:
​	Portfolio portfolio_;
​	};
​
​	TEST_F(APortfolio, IsEmptyWhenCreated) {
​	ASSERT_TRUE(portfolio_.IsEmpty());
​	}
​
​	TEST_F(APortfolio, IsNotEmptyAfterPurchase) {
​	portfolio_.Purchase(​"IBM"​, 1);
​
​	ASSERT_FALSE(portfolio_.IsEmpty());
​	}
​	TEST_F(APortfolio, AnswersZeroForShareCountOfUnpurchasedSymbol) {
​	ASSERT_THAT(portfolio_.ShareCount(​"AAPL"​), Eq(0u));
​	}
​
​	TEST_F(APortfolio, AnswersShareCountForPurchasedSymbol) {
​	portfolio_.Purchase(​"IBM"​, 2);
​	ASSERT_THAT(portfolio_.ShareCount(​"IBM"​), Eq(2u));
​	}

c6/1/Portfolio.h
​	#ifndef Portfolio_h
​	#define Portfolio_h
​
​	#include <string>
​
​	​class​ Portfolio {
​	​public​:
​	Portfolio();
​	​bool​ IsEmpty() ​const​;
​	​void​ Purchase(​const​ std::​string​& symbol, ​unsigned​ ​int​ shareCount);
​	​unsigned​ ​int​ ShareCount(​const​ std::​string​& symbol) ​const​;
​
​	​private​:
​	​bool​ isEmpty_;
​	​unsigned​ ​int​ shareCount_;
​	};
​
​	#endif

c6/1/Portfolio.cpp
​	#include "Portfolio.h"
​	​using​ ​namespace​ std;
​	Portfolio::Portfolio()
​	: isEmpty_{true}
​	, shareCount_{0u} {
​	}
​	​bool​ Portfolio::IsEmpty() ​const​ {
​	​return​ isEmpty_;
​	}
​
​	​void​ Portfolio::Purchase(​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount) {
​	isEmpty_ = false;
​	shareCount_ = shareCount;
​	}
​
​	​unsigned​ ​int​ Portfolio::ShareCount(​const​ ​string​& symbol) ​const​ {
​	​return​ shareCount_;
​	}

Were you able to understand what the Portfolio class does by reading the tests? You should be building the habit of reading test names as your first understanding of what a class has been designed to do. The tests are your gateway to understanding.

Simple Duplication in the Portfolio Manager

We have duplication in both our tests and our production code. The string literal "IBM" repeats three times across two tests: it appears once in IsNotEmptyAfterPurchase and twice in AnswersShareCountForPurchasedSymbol. Extracting the literal to a constant makes reading things a little easier, it reduces the risk of mistyping the literal in the future, and it makes new tests a little easier to write. Further, if the symbol for IBM needs to change, we can make that change in one place.

c6/2/PortfolioTest.cpp
​	#include "gmock/gmock.h"
​	#include "Portfolio.h"
​
​	​using​ ​namespace​ ::testing;
​	​using​ ​namespace​ std;
​
​	​class​ APortfolio: ​public​ Test {
​	​public​:
*	​static​ ​const​ ​string​ IBM;
​	Portfolio portfolio_;
​	};
*	​const​ ​string​ APortfolio::IBM(​"IBM"​);
​	​// ...​
​	TEST_F(APortfolio, IsEmptyWhenCreated) {
​	ASSERT_TRUE(portfolio_.IsEmpty());
​	}
​
​	TEST_F(APortfolio, IsNotEmptyAfterPurchase) {
​	portfolio_.Purchase(IBM, 1);
​	ASSERT_FALSE(portfolio_.IsEmpty());
​	}
​	​// ...​
​	TEST_F(APortfolio, AnswersZeroForShareCountOfUnpurchasedSymbol) {
​	ASSERT_THAT(portfolio_.ShareCount(​"AAPL"​), Eq(0u));
​	}
​	TEST_F(APortfolio, AnswersShareCountForPurchasedSymbol) {
​	portfolio_.Purchase(IBM, 2);
​
​	ASSERT_THAT(portfolio_.ShareCount(IBM), Eq(2u));
​	}
​
​	TEST_F(APortfolio, ThrowsOnPurchaseOfZeroShares) {
​	ASSERT_THROW(portfolio_.Purchase(IBM, 0), InvalidPurchaseException);
​	}

Does that mean you should always extract common literals to a variable? Suppose AnswersShareCountForPurchasedSymbol was the only test in which we needed the "IBM" literal. Creating a local variable IBM would have given us the benefits as claimed before. But the value of the variable in this case seems to be less. We can easily see all uses of the literal in a two-line test, so it’s trivial and safe to change both if needed.

Design choices are often judgment calls. Strive to adhere to the design principles set out in this chapter to better understand how they benefit your system. With that experience, when you encounter code that requires a judgment call, you’ll understand the implication of forgoing a design rule.

Take a look at the production code and see whether you can spot the duplication:

c6/1/Portfolio.cpp
​	#include "Portfolio.h"
​	​using​ ​namespace​ std;
​	Portfolio::Portfolio()
​	: isEmpty_{true}
​	, shareCount_{0u} {
​	}
​	​bool​ Portfolio::IsEmpty() ​const​ {
​	​return​ isEmpty_;
​	}
​
​	​void​ Portfolio::Purchase(​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount) {
​	isEmpty_ = false;
​	shareCount_ = shareCount;
​	}
​
​	​unsigned​ ​int​ Portfolio::ShareCount(​const​ ​string​& symbol) ​const​ {
​	​return​ shareCount_;
​	}

The code doesn’t exhibit visibly obvious line-for-line (or expression-for-expression) duplication. Instead, it contains algorithmic duplication. The IsEmpty member function returns the value of a bool that changes when Purchase gets called. Yet the notion of emptiness is directly tied to the number of shares, stored also when Purchase gets called. We can eliminate this conceptual duplication by eliminating the isEmpty_ variable and instead having IsEmpty ask about the number of shares.

c6/2/Portfolio.cpp
​	​bool​ Portfolio::IsEmpty() ​const​ {
*	​return​ 0 == shareCount_;
​	}

(Yes, it’s slightly awkward to ask for the number of shares overall in order to determine emptiness, but it’s correct for now, in other words, in the incremental sense. Writing what you know to be short-term code should trigger interesting thoughts, which might in turn translate to new tests. The problem with our implementation is that the portfolio will return empty if someone purchases zero shares of a symbol. Our definition of empty is whether the portfolio contains any symbols. So, is that empty? Or should we disallow such purchases? For the purposes of moving forward, we choose the latter and write a test named ThrowsOnPurchaseOfZeroShares.)

Algorithmic duplication—different ways of solving the same problem, or pieces of a problem—becomes a significant problem as your system grows. More often than not, the duplication morphs into unintentional variance, as changes to one implementation don’t get rolled into the other implementations.

Can We Really Stick to an Incremental Approach?

A bit of coding later, and we have the following tests written (just the test signature now, as you should be able to imagine what these tests look like)...

c6/3/PortfolioTest.cpp
​	TEST_F(APortfolio, IsEmptyWhenCreated) {
​	TEST_F(APortfolio, IsNotEmptyAfterPurchase) {
​	TEST_F(APortfolio, AnswersZeroForShareCountOfUnpurchasedSymbol) {
​	TEST_F(APortfolio, AnswersShareCountForPurchasedSymbol) {
​	TEST_F(APortfolio, ThrowsOnPurchaseOfZeroShares) {
​	TEST_F(APortfolio, AnswersShareCountForAppropriateSymbol) {
​	TEST_F(APortfolio, ShareCountReflectsAccumulatedPurchasesOfSameSymbol) {
​	TEST_F(APortfolio, ReducesShareCountOfSymbolOnSell) {
​	TEST_F(APortfolio, ThrowsWhenSellingMoreSharesThanPurchased) {

with the following implementation:

c6/3/Portfolio.cpp
​	#include "Portfolio.h"
​	​using​ ​namespace​ std;
​	​bool​ Portfolio::IsEmpty() ​const​ {
​	​return​ 0 == holdings_.size();
​	}
​	​void​ Portfolio::Purchase(​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount) {
​	​if​ (0 == shareCount) ​throw​ InvalidPurchaseException();
​	holdings_[symbol] = shareCount + ShareCount(symbol);
​	}
​	​void​ Portfolio::Sell(​const​ std::​string​& symbol, ​unsigned​ ​int​ shareCount) {
​	​if​ (shareCount > ShareCount(symbol)) ​throw​ InvalidSellException();
​	holdings_[symbol] = ShareCount(symbol) - shareCount;
​	}
​
​	​unsigned​ ​int​ Portfolio::ShareCount(​const​ ​string​& symbol) ​const​ {
​	​auto​ it = holdings_.find(symbol);
​	​if​ (it == holdings_.end()) ​return​ 0;
​	​return​ it->second;
​	}

We’re told a new story.

The story throws a wrench into our implementation—we’re not tracking individual purchases, and we’re not capturing the date on our signatures. This is where many developers question the wisdom of TDD. Had we spent additional time up front vetting the requirements, we might have figured out that we need to track the date of purchase. Our initial design could have incorporated that need.

The story seems to represent a good-sized change, one that might take a bit more than ten minutes. We must define a data structure to represent a purchase, change the signature on the method, supply a date from client code (right now, just our tests), populate the data structure appropriately, and store it.

Nah, let’s not do that...at least not all at once. Let’s see if we can proceed incrementally, seeking positive feedback every few minutes. One way to do that is to make assumptions. Let’s create a test that makes one purchase and then demonstrates that a corresponding record exists in the retrieved list of purchase records. Our assumption is that purchases are always made on a specific date. That makes our current task simpler because we can ignore the need to pass a date to Purchase.

c6/4/PortfolioTest.cpp
​	TEST_F(APortfolio, AnswersThePurchaseRecordForASinglePurchase) {
​	portfolio_.Purchase(SAMSUNG, 5);
​	​auto​ purchases = portfolio_.Purchases(SAMSUNG);
​
​	​auto​ purchase = purchases[0];
​	ASSERT_THAT(purchase.ShareCount, Eq(5u));
​	ASSERT_THAT(purchase.Date, Eq(Portfolio::FIXED_PURCHASE_DATE));
​	}

To get this test to pass, we don’t even need to associate the purchase record with the holdings_ data structure. Since our current assumption is that this works for only a single purchase, we can define a “global” purchase record collection for Portfolio.

c6/4/Portfolio.h
​	​struct​ PurchaseRecord {
​	PurchaseRecord(​unsigned​ ​int​ shareCount, ​const​ boost::gregorian::date& date)
​	: ShareCount(shareCount)
​	, Date(date) {
​	}
​
​	​unsigned​ ​int​ ShareCount;
​	boost::gregorian::date Date;
​	};
​
​	​class​ Portfolio {
​	​public​:
*	​static​ ​const​ boost::gregorian::date FIXED_PURCHASE_DATE;
​
​	​bool​ IsEmpty() ​const​;
​
​	​void​ Purchase(​const​ std::​string​& symbol, ​unsigned​ ​int​ shareCount);
​	​void​ Sell(​const​ std::​string​& symbol, ​unsigned​ ​int​ shareCount);
​
​	​unsigned​ ​int​ ShareCount(​const​ std::​string​& symbol) ​const​;
*	std::vector<PurchaseRecord> Purchases(​const​ std::​string​& symbol) ​const​;
​
​	​private​:
​	std::unordered_map<std::​string​, ​unsigned​ ​int​> holdings_;
*	std::vector<PurchaseRecord> purchases_;
​	};

c6/4/Portfolio.cpp
*	​const​ date Portfolio::FIXED_PURCHASE_DATE(date(2014, Jan, 1));
​
​	​void​ Portfolio::Purchase(​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount) {
​	​if​ (0 == shareCount) ​throw​ InvalidPurchaseException();
​	holdings_[symbol] = shareCount + ShareCount(symbol);
*	purchases_.push_back(PurchaseRecord(shareCount, FIXED_PURCHASE_DATE));
​	}
​
​	vector<PurchaseRecord> Portfolio::Purchases(​const​ ​string​& symbol) ​const​ {
​	​return​ purchases_;
​	}

That’s simple code, but it takes a few minutes to put into place, each minute inviting more possibilities for making dumb mistakes. It’s nice to get positive feedback that we’ve entered code correctly before moving on.

We created a production constant, FIXED_PURCHASE_DATE, to allow us to make quick, demonstrable progress. We know it’s bogus. Let’s get rid of it by removing our temporary but useful assumption that all purchases are on the same date.

c6/5/PortfolioTest.cpp
​	TEST_F(APortfolio, AnswersThePurchaseRecordForASinglePurchase) {
*	date dateOfPurchase(2014, Mar, 17);
*	portfolio_.Purchase(SAMSUNG, 5, dateOfPurchase);
​
​	​auto​ purchases = portfolio_.Purchases(SAMSUNG);
​
​	​auto​ purchase = purchases[0];
​	ASSERT_THAT(purchase.ShareCount, Eq(5u));
​	ASSERT_THAT(purchase.Date, Eq(dateOfPurchase));
​	}

Rather than having to fix all the other tests that call the Purchase member function, we can take a smaller step and default the date parameter.

c6/5/Portfolio.h
​	​void​ Purchase(
​	​const​ std::​string​& symbol,
​
​	​unsigned​ ​int​ shareCount,
​
​	​const​ boost::gregorian::date& transactionDate=
*	Portfolio::FIXED_PURCHASE_DATE);

c6/5/Portfolio.cpp
*	​void​ Portfolio::Purchase(
*	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	​if​ (0 == shareCount) ​throw​ InvalidPurchaseException();
​
​	holdings_[symbol] = shareCount + ShareCount(symbol);
*	purchases_.push_back(PurchaseRecord(shareCount, transactionDate));
​	}

Using a fixed date isn’t a valid long-term requirement (although defaulting to the current time might be), so we now want to eliminate the default parameter on Purchase. Unfortunately, we have at least a handful of tests that call Purchase without passing a date.

One solution is to add a date argument to the calls in all of the affected tests. That seems tedious. It also might violate the principle of test abstraction (see ​Test Abstraction​) for those tests—none of them cares about the purchase date.

That we need to change many tests at once, without changing the behavior they describe, tells us they contain duplication that we should have eliminated. What if we supply a fixture helper method that handles the call to Purchase and provides a default value for the date so that the tests need not specify it?

c6/6/PortfolioTest.cpp
​	​class​ APortfolio: ​public​ Test {
​	​public​:
​	​static​ ​const​ ​string​ IBM;
​	​static​ ​const​ ​string​ SAMSUNG;
​	Portfolio portfolio_;
​	​static​ ​const​ date ArbitraryDate;
​
*	​void​ Purchase(
*	​const​ ​string​& symbol,
*	​unsigned​ ​int​ shareCount,
*	​const​ date& transactionDate=APortfolio::ArbitraryDate) {
*	portfolio_.Purchase(symbol, shareCount, transactionDate);
*	}
​	};
​
​	TEST_F(APortfolio, ReducesShareCountOfSymbolOnSell) {
*	Purchase(SAMSUNG, 30);
​
​	portfolio_.Sell(SAMSUNG, 13);
​
​	ASSERT_THAT(portfolio_.ShareCount(SAMSUNG), Eq(30u - 13));
​	}
​
​	TEST_F(APortfolio, AnswersThePurchaseRecordForASinglePurchase) {
​	date dateOfPurchase(2014, Mar, 17);
*	Purchase(SAMSUNG, 5, dateOfPurchase);
​
​	​auto​ purchases = portfolio_.Purchases(SAMSUNG);
​
​	​auto​ purchase = purchases[0];
​	ASSERT_THAT(purchase.ShareCount, Eq(5u));
​	ASSERT_THAT(purchase.Date, Eq(dateOfPurchase));
​	}

One point of potential debate is that the helper function Purchase removes a bit of information from the tests—specifically, that it’s delegating to the portfolio_ instance. A first-time reader must navigate into the helper method to see just what it’s doing. But it’s a simple function and doesn’t bury key information that the reader will have a hard time remembering.

As a rule of thumb, avoid hiding Act (see ​Arrange-Act-Assert/Given-When-Then​) specifics. We can use the helper method when we need to make a purchase as part of setting up a test. But for tests where we’re specifically testing Purchase behavior, we should directly invoke it. Thus, ReducesShareCountOfSymbolOnSell can use the helper, since it makes a purchase as part of arranging the test. AnswersShareCountForPurchasedSymbol verifies purchase behavior, so it retains the direct call to portfolio_.Purchase.

c6/7/PortfolioTest.cpp
​	TEST_F(APortfolio, AnswersShareCountForPurchasedSymbol) {
*	portfolio_.Purchase(IBM, 2);
​	ASSERT_THAT(portfolio_.ShareCount(IBM), Eq(2u));
​	}
​
​	TEST_F(APortfolio, ReducesShareCountOfSymbolOnSell) {
*	Purchase(SAMSUNG, 30);
​
​	portfolio_.Sell(SAMSUNG, 13);
​	ASSERT_THAT(portfolio_.ShareCount(SAMSUNG), Eq(30u - 13));
​	}

Personally, I don’t like the inconsistencies this creates in the way the tests look. I’m OK with running everything through the helper method, as long as it’s a simple one-liner delegation, as it is in this case. If that bothers you, another solution is to simply include the date parameter in each of the tests and use a constant with a name like ArbitraryPurchaseDate.

We’ve been taking an incremental approach with very small steps. Does it cost us? You betcha! It often requires introducing small bits of code that we later remove—tiny bits of waste product.

In return, we get the much more valuable ability to make continual forward progress on creating well-designed, correct code. We don’t worry about tackling new, never-before-considered features—we use TDD to incorporate them as a similar series of small steps. The more we keep our code clean, the easier it is to make our changes.

More Duplication

After a bit of test and production code cleanup, our tests around the purchase record are short and sweet.

c6/8/PortfolioTest.cpp
​	TEST_F(APortfolio, AnswersThePurchaseRecordForASinglePurchase) {
​	Purchase(SAMSUNG, 5, ArbitraryDate);
​
​	​auto​ purchases = portfolio_.Purchases(SAMSUNG);
​	ASSERT_PURCHASE(purchases[0], 5, ArbitraryDate);
​	}
​
​	TEST_F(APortfolio, IncludesSalesInPurchaseRecords) {
​	Purchase(SAMSUNG, 10);
​	Sell(SAMSUNG, 5, ArbitraryDate);
​
​	​auto​ sales = portfolio_.Purchases(SAMSUNG);
​	ASSERT_PURCHASE(sales[1], -5, ArbitraryDate);
​	}

To support the negative amounts in the purchase record, we changed the ShareCount member to a signed integer.

c6/8/Portfolio.h
​	​struct​ PurchaseRecord {
*	PurchaseRecord(​int​ shareCount, ​const​ boost::gregorian::date& date)
​	: ShareCount(shareCount)
​	, Date(date) {}
*	​int​ ShareCount;
​	boost::gregorian::date Date;
​	};

c6/8/PortfolioTest.cpp
​	​void​ ASSERT_PURCHASE(
*	PurchaseRecord& purchase, ​int​ shareCount, ​const​ date& date) {
​	ASSERT_THAT(purchase.ShareCount, Eq(shareCount));
​	ASSERT_THAT(purchase.Date, Eq(date));
​	}

The production code for the two transaction functions, Purchase and Sell, is already looking a bit dense in just three lines each.

c6/8/Portfolio.cpp
​	​void​ Portfolio::Purchase(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	​if​ (0 == shareCount) ​throw​ InvalidPurchaseException();
​	holdings_[symbol] = shareCount + ShareCount(symbol);
​	purchases_.push_back(PurchaseRecord(shareCount, transactionDate));
​	}
​
​	​void​ Portfolio::Sell(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	​if​ (shareCount > ShareCount(symbol)) ​throw​ InvalidSellException();
​	holdings_[symbol] = ShareCount(symbol) - shareCount;
​	purchases_.push_back(PurchaseRecord(-shareCount, transactionDate));
​	}

Further, they’re fairly similar in nature. We haven’t yet coded the proper logic to associate the purchase records with the appropriate symbol (perhaps using a hash table), and when we do, we don’t want to have to code it twice. Let’s see what duplication we can eliminate.

The Purchase and Sell functions have small variances between each of the three lines. Let’s take a look at each line in turn and see whether we can make them similar. The first line in each is a guard clause that enforces a constraint: sales cannot be for more shares than held, and purchases cannot be for zero shares. But shouldn’t sales have the same constraint—that you cannot sell zero shares? Our customer says yes.

A slight problem is that the exception type name InvalidPurchaseException is inappropriate for use in the Sell function. Let’s make it something more specific that both functions can use—ShareCountCannotBeZeroException.

c6/9/PortfolioTest.cpp
​	TEST_F(APortfolio, ThrowsOnPurchaseOfZeroShares) {
​	ASSERT_THROW(Purchase(IBM, 0), ShareCountCannotBeZeroException);
​	}
​	​// ...​
​	TEST_F(APortfolio, ThrowsOnSellOfZeroShares) {
​	ASSERT_THROW(Sell(IBM, 0), ShareCountCannotBeZeroException);
​	}

Both transaction methods end up with the same guard clause.

c6/9/Portfolio.cpp
​	​void​ Portfolio::Purchase(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
*	​if​ (0 == shareCount) ​throw​ ShareCountCannotBeZeroException();
​	holdings_[symbol] = shareCount + ShareCount(symbol);
​	purchases_.push_back(PurchaseRecord(shareCount, transactionDate));
​	}
​
​	​void​ Portfolio::Sell(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	​if​ (shareCount > ShareCount(symbol)) ​throw​ InvalidSellException();
*	​if​ (0 == shareCount) ​throw​ ShareCountCannotBeZeroException();
​	holdings_[symbol] = ShareCount(symbol) - shareCount;
​	purchases_.push_back(PurchaseRecord(-shareCount, transactionDate));
​	}

Moving on to the next line in each transaction method, we update the holdings entry for the appropriate symbol, by either adding to or subtracting from the existing shares for the symbol. But subtraction is the same as adding the inverse.

Let’s introduce a signed variable called shareChange to capture the inverse. Note that we can also use it in the final line of code (where we add the purchase record).

c6/10/Portfolio.cpp
​	​void​ Portfolio::Sell(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	​if​ (shareCount > ShareCount(symbol)) ​throw​ InvalidSellException();
​	​if​ (0 == shareCount) ​throw​ ShareCountCannotBeZeroException();
*	​int​ shareChange = -shareCount;
*	holdings_[symbol] = ShareCount(symbol) + shareChange;
*	purchases_.push_back(PurchaseRecord(shareChange, transactionDate));
​	}

Now we bounce back to Purchase and try to make it look more like Sell.

c6/11/Portfolio.cpp
​	​void​ Portfolio::Purchase(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	​if​ (0 == shareCount) ​throw​ ShareCountCannotBeZeroException();
*	​int​ shareChange = shareCount;
*	holdings_[symbol] = ShareCount(symbol) + shareChange;
*	purchases_.push_back(PurchaseRecord(shareChange, transactionDate));
​	}

We now have two lines of code at the end of each function that duplicate each other, plus the same guard clause in each. Let’s move the initialization of shareChange up a line to above the guard clause. Our tests will ensure that it’s a safe move, since moving lines up or down is highly risky.

We end up with three common lines at the end of each function. We also rename the use of shareCount in the guard clause to shareChange so that all three to-be-extracted lines use a common variable.

c6/12/Portfolio.cpp
​	​void​ Portfolio::Purchase(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	​int​ shareChange = shareCount;
*	​if​ (0 == shareChange) ​throw​ ShareCountCannotBeZeroException();
*	holdings_[symbol] = ShareCount(symbol) + shareChange;
*	purchases_.push_back(PurchaseRecord(shareChange, transactionDate));
​	}
​
​	​void​ Portfolio::Sell(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	​if​ (shareCount > ShareCount(symbol)) ​throw​ InvalidSellException();
​	​int​ shareChange = -shareCount;
*	​if​ (0 == shareChange) ​throw​ ShareCountCannotBeZeroException();
*	holdings_[symbol] = ShareCount(symbol) + shareChange;
*	purchases_.push_back(PurchaseRecord(shareChange, transactionDate));
​	}

Finally, we extract.

c6/13/Portfolio.cpp
​	​void​ Portfolio::Purchase(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	Transact(symbol, shareCount, transactionDate);
​	}
​
​	​void​ Portfolio::Sell(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
​	​if​ (shareCount > ShareCount(symbol)) ​throw​ InvalidSellException();
​	Transact(symbol, -shareCount, transactionDate);
​	}
​	​void​ Portfolio::Transact(
​	​const​ ​string​& symbol, ​int​ shareChange, ​const​ date& transactionDate) {
​	​if​ (0 == shareChange) ​throw​ ShareCountCannotBeZeroException();
​	holdings_[symbol] = ShareCount(symbol) + shareChange;
​	purchases_.push_back(PurchaseRecord(shareChange, transactionDate));
​	}

One more little expressiveness thing is that the name of our exception type InvalidSellException is not very good. Let’s change it to InsufficientSharesException.

c6/14/PortfolioTest.cpp
​	TEST_F(APortfolio, ThrowsWhenSellingMoreSharesThanPurchased) {
*	ASSERT_THROW(Sell(SAMSUNG, 1), InsufficientSharesException);
​	}

c6/14/Portfolio.cpp
​	​void​ Portfolio::Sell(
​	​const​ ​string​& symbol, ​unsigned​ ​int​ shareCount, ​const​ date& transactionDate) {
*	​if​ (shareCount > ShareCount(symbol)) ​throw​ InsufficientSharesException();
​	Transact(symbol, -shareCount, transactionDate);
​	}

Is there anything else we could do from the stance of our two simple design rules? It appears that we’ve squashed all the duplication. What of readability? Purchase does nothing other than delegate, so it’s clear, and Sell simply adds a constraint and reverses the shares, so it too makes immediate sense. Transact doesn’t quite have the immediacy we want.

Benefits of Small Methods

To read Transact, we must slow down and carefully pick out what each line is really trying to accomplish. The first line throws an exception if the change in the number of shares is zero. The second line obtains the shares for the symbol, adds the share change, and assigns it to an appropriate entry in the hashtable. The third line creates a purchase record and adds it to the overall list of purchases.

Transact consists of three simple one-liners. But if all one-liners require that sort of meticulous reading, then your system overall will be all that more difficult to navigate. It’s simply not expressive enough. Let’s fix that.

c6/15/Portfolio.cpp
​	​void​ Portfolio::Transact(
​	​const​ ​string​& symbol, ​int​ shareChange, ​const​ date& transactionDate) {
​	ThrowIfShareCountIsZero(shareChange);
​	UpdateShareCount(symbol, shareChange);
​	AddPurchaseRecord(shareChange, transactionDate);
​	}
​	​void​ Portfolio::ThrowIfShareCountIsZero(​int​ shareChange) ​const​ {
​	​if​ (0 == shareChange) ​throw​ ShareCountCannotBeZeroException();
​	}
​
​	​void​ Portfolio::UpdateShareCount(​const​ ​string​& symbol, ​int​ shareChange) {
​	holdings_[symbol] = ShareCount(symbol) + shareChange;
​	}
​
​	​void​ Portfolio::AddPurchaseRecord(​int​ shareChange, ​const​ date& date) {
​	purchases_.push_back(PurchaseRecord(shareChange, date));
​	}

I have amazing extra-sensory powers. Back in time, in early 2013 as I write this chapter, I can see the future faces of many readers. I sense consternation. And I understand it.

Here are the reasons you might offer for not doing what we just did:

• It’s extra effort. Creating new functions is a pain.

• Creating a function for a one-liner used in only one place seems ridiculous.

• The additional function calls incur performance overhead.

• It’s harder to follow the entire flow through all the code.

• You’ll end up with tens of thousands of little crummy methods, each with horribly long names.

And here are some reasons why you should consider moving your code in this direction:

• It adheres to the simple design rule of expressiveness. The code requires no explanatory comments. Its functions, each consisting of either one detailed line or a few declarative statements, are immediately understood. Problems stand out like sore thumbs in such small functions.

  In contrast, most systems have lots of dense, long functions that take far too long to comprehend. Defects hide easily in these functions.

• It adheres to the design concept of cohesion and the SRP. All lines of code in a given function are at the same level of abstraction. Each function has one reason to change.

• It paves the way for simpler future design changes. We still need to associate purchase records with the proper symbol. We can now do that in one place, as opposed to two. If we need to specialize AddPurchaseRecord, we’re ready to go. If we need to create a more sophisticated purchase record subsystem, we can quickly move the existing logic to a new class. If we need to support undo and redo or additional sophistications around purchasing and selling, we’re poised to factor into a command pattern with a base class of Transaction.

• Following the flow of code is more easily done without implementation details in the way. Transact acts as a declaration of policy. The helper methods—ThrowIfShareCountIsZero, UpdateShareCount, and AddPurchaseRecord—are implementation details you don’t need to know most of the time. Think about the notion of separating interface from implementation or separating abstractions from concrete details.

• The performance overhead of extract methods in this fashion is almost never a problem. See ​TDD and Performance​.

• Small functions represent the start of real reuse. As you extract more similarly small functions, you will begin to more readily spot duplicate concepts and constructs in your development efforts. You won’t end up with an unmanageable mass of tiny methods. You will instead shrink your production code size dramatically.

Enough preaching. While you might not be ready to embrace this drastic change in style, you should be willing to at least give it an honest effort as you practice TDD more. Be willing to move in the direction of smaller functions and see what happens.

Finishing the Functionality

We’re not done. A portfolio can return a list of purchase records, but only for a single symbol. Our next test requires the portfolio to answer the correct set of purchase records when multiple symbols have been purchased.

c6/16/PortfolioTest.cpp
​	​bool​ ​operator​==(​const​ PurchaseRecord& lhs, ​const​ PurchaseRecord& rhs) {
​	​return​ lhs.ShareCount == rhs.ShareCount && lhs.Date == rhs.Date;
​	}
​
​	TEST_F(APortfolio, SeparatesPurchaseRecordsBySymbol) {
​	Purchase(SAMSUNG, 5, ArbitraryDate);
​	Purchase(IBM, 1, ArbitraryDate);
​
​	​auto​ sales = portfolio_.Purchases(SAMSUNG);
​	ASSERT_THAT(sales, ElementsAre(PurchaseRecord(5, ArbitraryDate)));
​	}

Google Mock provides the ElementsAre matcher for verifying explicit elements in a collection. The comparison requires the ability to compare two PurchaseRecord objects, so we add an appropriate implementation for operator==. (We might also have chosen to implement operator== as a member function on PurchaseRecord, but currently we only have need for it in a test.) The test initially fails, since the purchases_ vector holds onto two purchase records—one for Samsung, one for IBM.

To get the test to pass, we first declare the purchaseRecords_ member variable in Portfolio.h, an unordered map that stores a vector of PurchaseRecord objects for each symbol. We also change the signature of AddPurchaseRecord to take a symbol.

c6/16/Portfolio.h
​	​class​ Portfolio {
​
​	​public​:
​	​bool​ IsEmpty() ​const​;
​	​void​ Purchase(
​	​const​ std::​string​& symbol,
​	​unsigned​ ​int​ shareCount,
​	​const​ boost::gregorian::date& transactionDate);
​	​void​ Sell(​const​ std::​string​& symbol,
​	​unsigned​ ​int​ shareCount,
​	​const​ boost::gregorian::date& transactionDate);
​	​unsigned​ ​int​ ShareCount(​const​ std::​string​& symbol) ​const​;
​	std::vector<PurchaseRecord> Purchases(​const​ std::​string​& symbol) ​const​;
​
​	​private​:
​	​void​ Transact(​const​ std::​string​& symbol,
​	​int​ shareChange,
​	​const​ boost::gregorian::date&);
​	​void​ UpdateShareCount(​const​ std::​string​& symbol, ​int​ shareChange);
*	​void​ AddPurchaseRecord(
*	​const​ std::​string​& symbol,
*	​int​ shareCount,
*	​const​ boost::gregorian::date&);
​	​void​ ThrowIfShareCountIsZero(​int​ shareChange) ​const​;
​
​	std::unordered_map<std::​string​, ​unsigned​ ​int​> holdings_;
​	std::vector<PurchaseRecord> purchases_;
*	std::unordered_map<std::​string​, std::vector<PurchaseRecord>> purchaseRecords_;
​	};

We correspondingly update the implementation of Transact to pass the symbol to AddPurchaseRecord. In AddPurchaseRecord, we write new code that adds a PurchaseRecord to the purchaseRecords_ map (first inserting an empty vector if needed). We leave the existing logic that adds to the purchases_ vector untouched—we want to get our new code working before we worry about cleaning up old code.

c6/16/Portfolio.cpp
​	​void​ Portfolio::Transact(
​	​const​ ​string​& symbol, ​int​ shareChange, ​const​ date& transactionDate) {
​	ThrowIfShareCountIsZero(shareChange);
​	UpdateShareCount(symbol, shareChange);
*	AddPurchaseRecord(symbol, shareChange, transactionDate);
​	}
​
​	​void​ Portfolio::AddPurchaseRecord(
​	​const​ ​string​& symbol, ​int​ shareChange, ​const​ date& date) {
​	purchases_.push_back(PurchaseRecord(shareChange, date));
*	​auto​ it = purchaseRecords_.find(symbol);
*	​if​ (it == purchaseRecords_.end())
*	purchaseRecords_[symbol] = vector<PurchaseRecord>();
*	purchaseRecords_[symbol].push_back(PurchaseRecord(shareChange, date));
​	}
​
​	​unsigned​ ​int​ Portfolio::ShareCount(​const​ ​string​& symbol) ​const​ {
​	​auto​ it = holdings_.find(symbol);
​	​if​ (it == holdings_.end()) ​return​ 0;
​	​return​ it->second;
​	}
​	vector<PurchaseRecord> Portfolio::Purchases(​const​ ​string​& symbol) ​const​ {
*	​// return purchases_;​
*	​return​ purchaseRecords_.find(symbol)->second;
​	}

In the Purchases function, we return the vector of purchase records corresponding to the symbol. We write just enough code, not worrying yet about the possibility that the symbol is not found. Instead of worrying, we add an entry (“deal with symbol not found in Purchases”) to our test list.

Once our tests all pass, we clean up our code by removing references to purchases_. We write the test we just added to our test lists. We clean things up a bit more. Expressiveness-wise, AddPurchaseRecord is a bit dense. Duplication-wise, ShareCount and Purchases contain redundant code around finding elements from a map. We fix both problems.

c6/17/PortfolioTest.cpp
​	TEST_F(APortfolio, AnswersEmptyPurchaseRecordVectorWhenSymbolNotFound) {
​	ASSERT_THAT(portfolio_.Purchases(SAMSUNG), Eq(vector<PurchaseRecord>()));
​	}

c6/17/Portfolio.h
​	​template​<​typename​ T>
​	T Find(std::unordered_map<std::​string​, T> map, ​const​ std::​string​& key) ​const​ {
​	​auto​ it = map.find(key);
​	​return​ it == map.end() ? T{} : it->second;
​	}

c6/17/Portfolio.cpp
​	#include "Portfolio.h"
​	#include "PurchaseRecord.h"
​	​using​ ​namespace​ std;
​	​using​ ​namespace​ boost::gregorian;
​
​	​bool​ Portfolio::IsEmpty() ​const​ {
​	​return​ 0 == holdings_.size();
​	}
​
​	​void​ Portfolio::Purchase(
​	​const​ ​string​& symbol,
​	​unsigned​ ​int​ shareCount,
​	​const​ date& transactionDate) {
​	Transact(symbol, shareCount, transactionDate);
​	}
​
​	​void​ Portfolio::Sell(
​	​const​ ​string​& symbol,
​	​unsigned​ ​int​ shareCount,
​	​const​ date& transactionDate) {
​	​if​ (shareCount > ShareCount(symbol)) ​throw​ InvalidSellException();
​	Transact(symbol, -shareCount, transactionDate);
​	}
​
​	​void​ Portfolio::Transact(
​	​const​ ​string​& symbol, ​int​ shareChange, ​const​ date& transactionDate) {
​	ThrowIfShareCountIsZero(shareChange);
​	UpdateShareCount(symbol, shareChange);
​	AddPurchaseRecord(symbol, shareChange, transactionDate);
​	}
​
​	​void​ Portfolio::ThrowIfShareCountIsZero(​int​ shareChange) ​const​ {
​	​if​ (0 == shareChange) ​throw​ ShareCountCannotBeZeroException();
​	}
​
​	​void​ Portfolio::UpdateShareCount(​const​ ​string​& symbol, ​int​ shareChange) {
​	holdings_[symbol] = ShareCount(symbol) + shareChange;
​	}
​
​	​void​ Portfolio::AddPurchaseRecord(
​	​const​ ​string​& symbol, ​int​ shareChange, ​const​ date& date) {
​	​if​ (!ContainsSymbol(symbol))
​	InitializePurchaseRecords(symbol);
​	Add(symbol, {shareChange, date});
​	}
​
​	​void​ Portfolio::InitializePurchaseRecords(​const​ ​string​& symbol) {
​	purchaseRecords_[symbol] = vector<PurchaseRecord>();
​	}
​	​void​ Portfolio::Add(​const​ ​string​& symbol, PurchaseRecord&& record) {
​	purchaseRecords_[symbol].push_back(record);
​	}
​
​	​bool​ Portfolio::ContainsSymbol(​const​ ​string​& symbol) ​const​ {
​	​return​ purchaseRecords_.find(symbol) != purchaseRecords_.end();
​	}
​
​	​unsigned​ ​int​ Portfolio::ShareCount(​const​ ​string​& symbol) ​const​ {
​	​return​ Find<​unsigned​ ​int​>(holdings_, symbol);
​	}
​
​	vector<PurchaseRecord> Portfolio::Purchases(​const​ ​string​& symbol) ​const​ {
​	​return​ Find<vector<PurchaseRecord>>(purchaseRecords_, symbol);
​	}

Well, we didn’t clean things up just a “bit,” did we? We once again did dramatic, lots-of-small-functions refactoring. AddPurchaseRecord now declares high-level policy, and each of the three functions representing steps in that policy encapsulates details. Overkill? Perhaps. Benefits? Immediacy of comprehension, certainly. The isolation of implementation details also means that if we wanted to use a different data structure, our changes would be easier to spot and also isolated, thus diminishing risk. Also, we can clearly spot each of the steps that alters the state of the Portfolio because of our use of const on appropriate member functions.

Finally, we poised ourselves for a better design. In the next section, our collection of purchase records ends up a first-level class on its own. Our current cleanup of encapsulating all operations on this collection provides for an easier transition to that new design.

To be clear, we aren’t prethinking our design. Instead, we get an increment of code to work and then seek to optimize the design of the current solution. The side effect is that subsequent changes are easier.

Incremental Design Made Simple

Our Portfolio contains two collections that parallel each other: holdings_ maps the symbol to a total of shares, and purchaseRecords_ maps the symbol to a list of purchase records. We could eliminate holdings_ and instead calculate the total of shares for a given symbol on demand.

Keeping two collections represents a performance optimization. It results in slightly more complex code, and we need to ensure that the two collections always match each other. That’s ultimately your call. If you think you need the performance, keep things the way they are. We don’t need it yet, so we’ll factor out the common code.

The first step is to change the ShareCount function to dynamically calculate the number of shares for a given symbol.

c6/18/Portfolio.cpp
​	​unsigned​ ​int​ Portfolio::ShareCount(​const​ ​string​& symbol) ​const​ {
​	​auto​ records = Find<vector<PurchaseRecord>>(purchaseRecords_, symbol);
​	​return​ accumulate(records.begin(), records.end(), 0,
​	[] (​int​ total, PurchaseRecord record) {
​	​return​ total + record.ShareCount; });
​	}

We no longer need to make a call to UpdateShareCount from Transact. We can safely delete UpdateShareCount! We then change IsEmpty to refer to purchaseRecords_ instead of holdings_, which allows us to finally delete the declaration for holdings_.

c6/18/Portfolio.cpp
​	​bool​ Portfolio::IsEmpty() ​const​ {
*	​return​ 0 == purchaseRecords_.size();
​	}
​
​	​void​ Portfolio::Transact(
​	​const​ ​string​& symbol, ​int​ shareChange, ​const​ date& transactionDate) {
​	ThrowIfShareCountIsZero(shareChange);
​	AddPurchaseRecord(symbol, shareChange, transactionDate);
​	}

That was easy enough.

The final effort is to move all code related to the collection of purchase records to a separate class. Why? The Portfolio class is violating the SRP. Its primary reason to change is any modification to the policy of how we manage the portfolio. But it has an additional reason to change—implementation specifics around the collection of purchase records.

So what? Well, we just made a design change that simplifies our code, but that change could also represent an unacceptable performance degradation. Isolating the purchase record code would represent an SRP-compliant design. It would allow us to more easily pinpoint where our performance change should go. Extracting the code would decrease our chance of accidentally breaking something else in Portfolio.

We can once again make this change incrementally, adding a bit of new code and running our tests to ensure things still work. The first step is to introduce a new member variable that maps symbols to holdings. We can name it holdings_ (hey, that sounds familiar!).

c6/19/Portfolio.h
*	std::unordered_map<std::​string​, Holding> holdings_;

Next, we incrementally start adding parallel support to update the holdings_ map, starting first in InitializePurchaseRecords.

c6/19/Portfolio.cpp
​	​void​ Portfolio::InitializePurchaseRecords(​const​ ​string​& symbol) {
​	purchaseRecords_[symbol] = vector<PurchaseRecord>();
*	holdings_[symbol] = Holding();
​	}

In the Add function, we delegate to a function with the same name in the Holding class. We obtain the code for the Holding class by copying it over from Portfolio and simplifying it appropriately.

c6/19/Portfolio.cpp
​	​void​ Portfolio::Add(​const​ ​string​& symbol, PurchaseRecord&& record) {
​	purchaseRecords_[symbol].push_back(record);
*	holdings_[symbol].Add(record);
​	}

c6/19/Holding.h
​	​void​ Add(PurchaseRecord& record) {
​	purchaseRecords_.push_back(record);
​	}
​	std::vector<PurchaseRecord> purchaseRecords_;

On to the Purchases function. We replace the existing code (now commented out—don’t worry, these commented lines won’t stick around in our code) with a simpler version, again delegating to a new function defined on Holding.

c6/19/Portfolio.cpp
​	vector<PurchaseRecord> Portfolio::Purchases(​const​ ​string​& symbol) ​const​ {
​	​// return Find<vector<PurchaseRecord>>(purchaseRecords_, symbol);​
​	​return​ Find<Holding>(holdings_, symbol).Purchases();
​	}

c6/19/Holding.h
​	std::vector<PurchaseRecord> Purchases() ​const​ {
​	​return​ purchaseRecords_;
​	}

We update ContainsSymbol by asking the same question of holdings_ as we previously did of the purchaseRecords_ collection.

c6/19/Portfolio.cpp
​	​bool​ Portfolio::ContainsSymbol(​const​ ​string​& symbol) ​const​ {
​	​// return purchaseRecords_.find(symbol) != purchaseRecords_.end();​
​	​return​ holdings_.find(symbol) != holdings_.end();
​	}

Changing ShareCount is another delegation effort.

c6/19/Portfolio.cpp
​	​unsigned​ ​int​ Portfolio::ShareCount(​const​ ​string​& symbol) ​const​ {
​	​// auto records = Find<vector<PurchaseRecord>>(purchaseRecords_, symbol);​
​	​// return accumulate(records.begin(), records.end(), 0,​
​	​// [] (int total, PurchaseRecord record) {​
​	​// return total + record.ShareCount; });​
​	​return​ Find<Holding>(holdings_, symbol).ShareCount();
​	}

c6/19/Holding.h
​	​unsigned​ ​int​ ShareCount() ​const​ {
​	​return​ accumulate(purchaseRecords_.begin(), purchaseRecords_.end(), 0,
​	[] (​int​ total, PurchaseRecord record) {
​	​return​ total + record.ShareCount; });
​	}

Last, we try to remove the purchaseRecords_ member variable. The compiler tells us which code still references the variable. We delete those references and make sure our tests still run. (They do!)

c6/19/Portfolio.h
​	​// no longer needed!​
​	std::unordered_map<std::​string​, std::vector<PurchaseRecord>> purchaseRecords_;

We’re almost done. The last job is to ensure that we’ve written tests for the Holding class. The code is already tested, in the context of Portfolio, so why add tests? The reason is that we also want the documentation value that tests can provide. If a developer wants to use the Holding class, they should be able to understand how it’s used by reading tests that directly document its behavior.

When extracting a new class in this fashion, you’ll sometimes be able to move tests directly across (for example, from PortfolioTest to HoldingTest). The tests often simplify, and you’ll likely need to reconsider their name. (Look at the source download to see the final tests.)

The end result is that the code in Holding is excruciatingly simple, all one-liners, and immediately understood. The code in Portfolio is also fairly simple, all one- or two-liners, each immediately understood.

Another beautiful thing throughout this whole process is that we were able to make dramatic changes to our design, bit by bit, without worrying. I’ll be honest, while initially coding this example for the book, I made at least one dumb mistake and was happy to have tests that immediately let me know.

As far as performance goes, getting back the original performance would be straightforward. If needed, we could cache the share total in a member variable defined on Holding, add to that total on each call to Add, and simply return that value from ShareCount.