Chapter 13. The Sniper Makes a Bid
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.
Introducing AuctionSniper
A New Class, with Dependencies
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.
Figure 13.1 Plugging in the AuctionSniper
Focus, Focus, Focus
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.
Sending a Bid
An Auction Interface
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.
Figure 13.2 Introducing Auction
The AuctionSniper Bids
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.
How Should We Describe Expected Values?
We’ve specified the expected bid value by adding the price and increment. There are different opinions about whether test values should just be literals with “obvious” values, or expressed in terms of the calculation they represent. Writing out the calculation may make the test more readable but risks reimplementing the target code in the test, and in some cases the calculation will be too complicated to reproduce. Here, we decide that the calculation is so trivial that we can just write it into the test.
jMock Expectations Don’t Need to Be Matched in Order
This is our first test with more than one expectation, so we’ll point out that the order in which expectations are declared does not have to match the order in which the methods are called in the code. If the calling order does matter, the expectations should include a sequence clause, which is described in Appendix A.
The implementation to make the test pass is simple.
Successfully Bidding with the AuctionSniper
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.
Expressing Intent in API
We were only able to discover that we could pass null as a MessageListener because we have the source code to the Smack library. This isn’t clear from the API because, presumably, the authors wanted to enforce the right behavior and it’s not clear why anyone would want a Chat without a listener. An alternative would have been to provide equivalent creation methods that don’t take a listener, but that would lead to API bloat. There isn’t an obvious best approach here, except to note that including well-structured source code with the distribution makes libraries much easier to work with.
Now we can restructure our connection code and use the Chat to send back a bid.
Null Implementation
A null implementation is similar to a null object [Woolf98]: both are implementations that respond to a protocol by not doing anything—but the intention is different. A null object is usually one implementation amongst many, introduced to reduce complexity in the code that calls the protocol. We define a null implementation as a temporary empty implementation, that allows the programmer to make progress by deferring effort. It will be replaced be a real implementation before we ship.
The End-to-End Tests Pass
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.
Tidying Up the Implementation
Extracting XMPPAuction
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.
Figure 13.4 Closing the loop with an XMPPAuction
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.
Extracting the User Interface
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.
Figure 13.5 Extracting SniperStateDisplayer
Tidying Up the Translator
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.
Encapsulate Collections
We’ve developed a habit of packaging up common types, such as collections, in our own classes, even though Java generics avoid the need to cast objects. We’re trying to use the language of the problem we’re working on, rather than the language of Java constructs. In our two versions of processMessage(), the first has lots of incidental noise about looking up and parsing values. The second is written in terms of auction events, so there’s less of a conceptual gap between the domain and the code.
Our rule of thumb is that we try to limit passing around types with generics (the types enclosed in angle brackets). Particularly when applied to collections, we view it as a form of duplication. It’s a hint that there’s a domain concept that should be extracted into a type.
Defer Decisions
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.
Keep the Code Compiling
We try to minimize the time when we have code that does not compile by keeping changes incremental. When we have compilation failures, we can’t be quite sure where the boundaries of our changes are, since the compiler can’t tell us. This, in turn, means that we can’t check in to our source repository, which we like to do often. The more code we have open, the more we have to keep in our heads which, ironically, usually means we move more slowly. One of the great discoveries of test-driven development is just how fine-grained our development steps can be.
Emergent Design
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.