Removing Duplication, Selection Troubles, and a Few Burrs

So where are we?

• Cost ” I'm reasonably happy with this class. There are other ways to do it, but I'll take it as fine for now.

• Card ” I'm still not thrilled about the cards/rollover stuff, but I'll leave it. Selection could be improved ”should the card notify its focus listeners when it's clicked? This would remove its dependency on Background.

• Background ” Other than changing Card and selection, I'm happy with it.

• PlanningGame ” There's still duplication in the setup. Colleague Ron Crocker pointed out that we could let makeCustomerButtons() and makeProgrammerButtons() take the responsibility for putting in the proper label. But the whole handling of containers and selection remains troublesome . And we're still calling updateCost() in a bunch of places.

Exercise 102 Button Creation.

Clean up button creation in PlanningGame.

• Move the labels into the "make" routines.

• Eliminate the duplication in constructing buttons .

See Appendix A for solutions.

I've been trying to understand why the selection handling bothers me so much. I think it's based on the responsibilities of the classes: I want the planning game screen to be responsible for holding the buttons, the Background, and the summary. Currently, the planning game screen also tracks the selection because that's what the action listeners work with. But I think instead we should make the Background track all selections; some of the action listener work belongs there too.

This change is a bit tentative, so I will certainly checkpoint my code so that I can return to it if I don't like where things end up. (I think of checkpointing as saving a version for easy recovery without releasing it to the whole team; this may or may not be easy in your environment.)

Exercise 103 Move Selection Handling to Background.

• Make Background hold the selection.

• Move appropriate listener behavior over to Background (at least from New, Split, and Delete).

• Move the ContainerListener.

See Appendix A for solutions.

Updating the cost is done by any button press that might change it. Each command must be careful to include a call to updateCost() after any changes it causes. In the current code, this is done by putting the call in each listener. One way to reduce this duplication is to subclass listeners from a common parent and make the parent responsible for the call.

An alternative would be to introduce listener-style notification; the summary would listen for changes in the underlying Background or velocity and would update itself accordingly . PropertyChangeEvent fits our needs. On Background, the two things we typically monitor are the count and the cost. Rather than be specific about which property has changed, I'll use the standard bean convention that null for a property change means anything might have changed.

This change is a refactoring, but it's into the realm of large refactoring (or would be if this program were of a substantial size). I don't feel like my tests will adequately cover it, so I'll introduce new tests to make sure Background sends events. Since this is a relatively big change, checkpoint before you start.

Exercise 104 Property Changes.

Transform PlanningGame so cost updates are triggered by property changes from Background rather than from explicit calls.

See Appendix A for solutions.

The action listeners still have duplication: Several of them need to check whether there are any cards before they do their work. This suggests a change: Why not disable them unless they apply? This is a user interface change, but our user likes the idea.

Exercise 105 Enable Buttons.

Make buttons enabled only when appropriate. (Test first, of course.)

See Appendix A for solutions.

Look back at Card. It still depends on Background. When a card is clicked, it looks at the background and tells the background to move that card to the front. This is an undesirable dependency because Background knows about Card, and Card knows about Background.

Exercise 106 Card and Background. (Challenging).

Remove Card's dependency on Background. Make Card send a property notification when it is clicked. (You could make the case for a different notification if you wanted.) Make Background listen for the new notification. Remove the ContainerListener aspect of Background. (Since Background now controls the moveToFront() call, it already knows when the contents are changing.) See Appendix A for solutions.

Exercise 107 Cleanup.

Go through your tests and code one more time, doing any simple cleanup you can. See Appendix A for solutions.

Now compare your code to the original. Does it communicate better? Is it simpler? Better structured?