In which we extract an AuctionSniper
class and tease out its dependencies. We plug our new class into the rest of the application, using an empty implementation of auction until we’re ready to start sending commands. We close the loop back to the auction house with an XMPPAuction
class. We continue to carve new types out of the code.
Our application accepts Price
events from the auction, but cannot interpret them yet. We need code that will perform two actions when the currentPrice()
method is called: send a higher bid to the auction and update the status in the user interface. We could extend Main
, but that class is looking rather messy—it’s already doing too many things at once. It feels like this is a good time to introduce what we should call an “Auction Sniper,” the component at the heart of our application, so we create an AuctionSniper
class. Some of its intended behavior is currently buried in Main
, and a good start would be to extract it into our new class—although, as we’ll see in a moment, it will take a little effort.
Given that an AuctionSniper
should respond to Price
events, we decide to make it implement AuctionEventListener
rather than Main
. The question is what to do about the user interface. If we consider moving this method:
does it really make sense for an AuctionSniper
to know about the implementation details of the user interface, such as the use of the Swing thread? We’d be at risk of breaking the “single responsibility” principle again. Surely an AuctionSniper
ought to be concerned with bidding policy and only notify status changes in its terms?
Our solution is to insulate the AuctionSniper
by introducing a new relationship: it will notify a SniperListener
of changes in its status. The interface and the first unit test look like this:
which says that Sniper should report that it has lost if it receives a Close
event from the auction.
The failure report says:
which we can make pass with a simple implementation:
Finally, we retrofit the new AuctionSniper
by having Main
implement SniperListener
.
Our working end-to-end test still passes and our broken one still fails at the same place, so we haven’t made things worse. The new structure looks like Figure 13.1.
Once again, we’ve noticed complexity in a class and used that to tease out a new concept from our initial skeleton implementation. Now we have a Sniper to respond to events from the translator. As you’ll see shortly, this is a better structure for expressing what the code does and for unit testing. We also think that the sniperLost()
method is clearer than its previous incarnation, auctionClosed()
, since there’s now a closer match between its name and what it does—that is, reports a lost auction.
Isn’t this wasteful fiddling, gold-plating the code while time slips by? Obviously we don’t think so, especially when we’re sorting out our ideas this early in the project. There are teams that overdo their design effort, but our experience is that most teams spend too little time clarifying the code and pay for it in maintenance overhead. As we’ve shown a couple of times now, the “single responsibility” principle is a very effective heuristic for breaking up complexity, and developers shouldn’t be shy about creating new types. We think Main
still does too much, but we’re not yet sure how best to break it up. We decide to push on and see where the code takes us.
The next step is to have the Sniper send a bid to the auction, so who should the Sniper talk to? Extending the SniperListener
feels wrong because that relationship is about tracking what’s happening in the Sniper, not about making external commitments. In the terms defined in “Object Peer Stereotypes” (page 52), SniperListener
is a notification, not a dependency.
After the usual discussion, we decide to introduce a new collaborator, an Auction
. Auction
and SniperListener
represent two different domains in the application: Auction
is about financial transactions, it accepts bids for items in the market; and SniperListener
is about feedback to the application, it reports changes to the current state of the Sniper. The Auction
is a dependency, for a Sniper cannot function without one, whereas the SniperListener
, as we discussed above, is not. Introducing the new interface makes the design look like Figure 13.2.
Now we’re ready to start bidding. The first step is to implement the response to a Price
event, so we start by adding a new unit test for the AuctionSniper
. It says that the Sniper, when it receives a Price
update, sends an incremented bid to the auction. It also notifies its listener that it’s now bidding, so we add a sniperBidding()
method. We’re making an implicit assumption that the Auction
knows which bidder the Sniper represents, so the Sniper does not have to pass in that information with the bid.
The failure report is:
When writing the test, we realized that we don’t actually care if the Sniper notifies the listener more than once that it’s bidding; it’s just a status update, so we use an atLeast(1)
clause for the listener’s expectation. On the other hand, we do care that we send a bid exactly once, so we use a one()
clause for its expectation. In practice, of course, we’ll probably only call the listener once, but this loosening of the conditions in the test expresses our intent about the two relationships. The test says that the listener is a more forgiving collaborator, in terms of how it’s called, than the Auction
. We also retrofit the atLeast(1)
clause to the other test method.
The implementation to make the test pass is simple.
Now we have to fold our new AuctionSniper
back into the application. The easy part is displaying the bidding status, the (slightly) harder part is sending the bid back to the auction. Our first job is to get the code through the compiler. We implement the new sniperBidding()
method on Main
and, to avoid having code that doesn’t compile for too long, we pass the AuctionSniper
a null implementation of Auction
.
So, what goes in the Auction
implementation? It needs access to the chat so it can send a bid message. To create the chat we need a translator, the translator needs a Sniper, and the Sniper needs an auction. We have a dependency loop which we need to break.
Looking again at our design, there are a couple of places we could intervene, but it turns out that the ChatManager
API is misleading. It does not require a MessageListener
to create a Chat
, even though the createChat()
methods imply that it does. In our terms, the MessageListener
is a notification; we can pass in null
when we create the Chat
and add a MessageListener
later.
Now we can restructure our connection code and use the Chat
to send back a bid.
Now the end-to-end tests pass: the Sniper can lose without making a bid, and lose after making a bid. We can cross off another item on the to-do list, but that includes just catching and printing the XMPPException
. Normally, we regard this as a very bad practice but we wanted to see the tests pass and get some structure into the code—and we know that the end-to-end tests will fail anyway if there’s a problem sending a message. To make sure we don’t forget, we add another to-do item to find a better solution, Figure 13.3.
Our end-to-end test passes, but we haven’t finished because our new implementation feels messy. We notice that the activity in joinAuction()
crosses multiple domains: managing chats, sending bids, creating snipers, and so on. We need to clean up. To start, we notice that we’re sending auction commands from two different levels, at the top and from within the Auction
. Sending commands to an auction sounds like the sort of thing that our Auction
object should do, so it makes sense to package that up together. We add a new method to the interface, extend our anonymous implementation, and then extract it to a (temporarily) nested class—for which we need a name. The distinguishing feature of this implementation of Auction
is that it’s based on the messaging infrastructure, so we call our new class XMPPAuction
.
We’re starting to see a clearer model of the domain. The line auction.join()
expresses our intent more clearly than the previous detailed implementation of sending a string to a chat. The new design looks like Figure 13.4 and we promote XMPPAuction
to be a top-level class.
We still think joinAuction()
is unclear, and we’d like to pull the XMPP-related detail out of Main
, but we’re not ready to do that yet. Another point to keep in mind.
The other activity in Main
is implementing the user interface and showing the current state in response to events from the Sniper. We’re not really happy that Main
implements SniperListener
; again, it feels like mixing different responsibilities (starting the application and responding to events). We decide to extract the SniperListener
behavior into a nested helper class, for which the best name we can find is SniperStateDisplayer
. This new class is our bridge between two domains: it translates Sniper events into a representation that Swing can display, which includes dealing with Swing threading. We plug an instance of the new class into the AuctionSniper
.
Figure 13.5 shows how we’ve reduced Main
so much that it no longer participates in the running application (for clarity, we’ve left out the WindowAdapter
that closes the connection). It has one job which is to create the various components and introduce them to each other. We’ve marked MainWindow
as external, even though it’s one of ours, to represent the Swing framework.
Finally, we fulfill our promise to ourselves and return to the AuctionMessageTranslator
. We start trying to reduce the noise by adding constants and static imports, with some helper methods to reduce duplication. Then we realize that much of the code is about manipulating the map of name/value pairs and is rather procedural. We can do a better job by extracting an inner class, AuctionEvent
, to encapsulate the unpacking of the message contents. We have confidence that we can refactor the class safely because it’s protected by its unit tests.
This is an example of “breaking out” that we described in “Value Types” (page 59). It may not be obvious, but AuctionEvent
is a value: it’s immutable and there are no interesting differences between two instances with the same contents. This refactoring separates the concerns within AuctionMessageTranslator
: the top level deals with events and listeners, and the inner object deals with parsing strings.
There’s a technique we’ve used a couple of times now, which is to introduce a null implementation of a method (or even a type) to get us through the next step. This helps us focus on the immediate task without getting dragged into thinking about the next significant chunk of functionality. The null Auction
, for example, allowed us to plug in a new relationship we’d discovered in a unit test without getting pulled into messaging issues. That, in turn, meant we could stop and think about the dependencies between our objects without the pressure of having a broken compilation.
What we hope is becoming clear from this chapter is how we’re growing a design from what looks like an unpromising start. We alternate, more or less, between adding features and reflecting on—and cleaning up—the code that results. The cleaning up stage is essential, since without it we would end up with an unmaintainable mess. We’re prepared to defer refactoring code if we’re not yet clear what to do, confident that we will take the time when we’re ready. In the meantime, we keep our code as clean as possible, moving in small increments and using techniques such as null implementation to minimize the time when it’s broken.
Figure 13.5 shows that we’re building up a layer around our core implementation that “protects” it from its external dependencies. We think this is just good practice, but what’s interesting is that we’re getting there incrementally, by looking for features in classes that either go together or don’t. Of course we’re influenced by our experience of working on similar codebases, but we’re trying hard to follow what the code is telling us instead of imposing our preconceptions. Sometimes, when we do this, we find that the domain takes us in the most surprising directions.
13.59.145.158