Design Review

Now that we’ve covered why the design and code review stage is so important, let’s get started with the review! For the purposes of this chapter, we’ll focus on the diagrams for the “Display Rollover Information” use case, because this use case happens to expose some areas where the design of the prototype code could be improved. Note that we’re not updating these diagrams in order to have a pristine document; we’re going to improve the code by revisiting the design. Along the way, we hope you’ll learn a bit about how to “model like you mean it,” as the team did.

“Display Rollover Information” Robustness Diagram Revisited

It quickly became obvious from looking at the sequence diagram for “Display Rollover Information” (see Figure 6-9) that the design was less than ideal. We traced the problem back to the robustness diagram and, in the course of reviewing it, discovered several issues that we felt could lead to a better design if addressed.

The robustness diagram that the team used as the basis for the design is shown back in Figure 6-8. In this section, we’ll critique this diagram, working through the improvements one by one. We’ll show some snapshots of the diagram as it’s being improved. And then, using our new and improved robustness diagram, we’ll create a new version of the sequence diagram and rework the code to match.

Step 1: “Decluttering” the Diagram

The robustness diagrams should be drawn at a fairly high level of abstraction so that they’re uncluttered and easy to follow. This means showing less detail than what we’ll be showing in the sequence diagrams. With this in mind, we can make several refinements in this diagram:

• All of the “On Click” and “On Mouse Over” controllers can be removed, and instead the controller name can simply be placed on the arrow. (We can safely do this, because, as drawn, these “On Click” controllers have no real behavior—the software behavior is in the controllers that are linked to these. As a general rule, if a controller has no behavior, don’t draw it.) For example, the On Click on Multiple Hotels controller (near the center of the diagram) could be removed, and we could instead have an arrow going from Map Viewer to “Get list of clicked hotels.” The arrow would then be labeled On Click on Multiple Hotels.

• Similarly, we can get rid of the On Mouse Over a Hotel Icon controller, by just labeling the arrow coming from the Map Viewer.

• Also, is the Change Mouse Icon operation major enough to warrant being on this diagram? We can combine Change Mouse Icon and Change Hotel Icon into something like “Change mouse icon and highlight Hotel icon” on the robustness diagram, even though these would be two different operations on a sequence diagram.

• We can also rename “mouse icon” to “cursor,” as the term “cursor” is more intuitive than “mouse icon.” So the renamed operation would be “Change cursor and highlight Hotel icon.”

The result of these analysis-model refactorings should be a robustness diagram that’s simpler and easier to read. The new and improved robustness diagram (so far) is shown in Figure 7-2.

Figure 7-2: New and improved “Display Rollover Information” robustness diagram. It’s looking better already!

We’ve also jiggled the objects around a bit to improve the readability, and as you can see, the straight-line police paid a visit to this diagram. Note, however, that in most cases it just isn’t worth spending ages perfecting a diagram’s layout. In certain cases when presentation is important (like, hey, for a book!), it’s worth doing, but usually the main purpose of a diagram is to get you to the next stage of development (in this case, the sequence diagrams), not to spend ages making every line perfectly straight.

On discussing these changes with the ESRI team, we realized that Change Mouse Icon is never really explicitly called. The cursor graphic is just a parameter of the HTML image map, and the browser automatically does this for us. However, the Change Hotel Icon operation is explicitly called when the onMouseOver or onMouseExit events occur for a particular image.

So at this stage, we could either revisit the diagram and change it again or leave this part of it unaltered. We opted to leave the operation as “Change cursor and highlight Hotel icon,” because that’s what actually happens. (At this stage, we’re not so concerned with how things happen—much of that thought process can be left to the design stage.)

Step 2: Disambiguating the Diagram

Now that the diagram has been “decluttered”, we instantly begin to see some other improvements and corrections (aka disambiguations) that we can make.

The two controllers at the top-right of Figure 7-2, Create Display Hotel Attribute Window and Show Display Hotel Attribute Window, could probably be merged into a single controller. Their purpose is similar enough that we don’t gain anything from them being shown separately. Remember that robustness diagrams are high-level analysis tools.^[3.]

While we’re at it, where did the Display Hotel Attribute Window name come from in the first place? It doesn’t appear in the domain model or the use case, so at this point alarm bells should be ringing—we have a rogue controller on our diagram! If we check back to the “Display Rollover Information” use case, the following sentence provides the answer:

The user may click the hotel icon to display a hotel information pop-up that displays the hotel’s attributes for all of the fields in the hotel pop-up fields collection.

So Show Display Hotel Attribute Window should be renamed as Show Hotel Info Pop-up. (You might think this sort of thing is too trivial to bother with, but it’s not. It’s amazing how many bugs get introduced into software because elements are named ambiguously.)

Tip 

Let’s boil this down to a simple rule of thumb. The use case text has to match the robustness diagram, and the robustness diagram has to match the class diagram, and the class diagram has to match the code. In short, that’s how you can drive code from use cases.

A question that arose during the design review was, “Should ‘Hotel Info Pop-up’ also appear on the domain model, as it’s an entity, a noun?” Although it should be a Boundary object on the robustness diagram, we chose not to put it on the domain model. Nouns for “GUI stuff” stay off the domain model because they’re solution space in nature (i.e., we would have built a different solution for hotel map finders that didn’t use this particular GUI widget). Another reason we avoid putting GUI stuff in the domain model is because it would very quickly get all cluttered up. Starting to include GUI stuff on the domain model is really opening Pandora’s box, and it should be avoided.

MapTip could also be a Boundary object on the robustness diagram. When you look at the finished diagram (skip ahead to Figure 7-4 if you can’t wait!), you’ll see how the robustness diagram really “tells the story” of the behavior requirements written in the use case.

OBJECT DISCOVERY IN ACTION: INTRODUCING MULTIPLEHOTELCLUSTER

At this point, the question arose about whether Hotels (the entity) should be Hotel (the domain class) or HotelCollection (the domain class). The issue arises because when a map is zoomed out far enough, a single icon is used to represent a cluster of hotels that are very close together, and the code needs to make this all work correctly (meaning the design needs to account for it). Our first inclination was that it should probably be HotelCollection, not Hotel (because it was providing a list of hotels). We were seeing basically the same disconnect error between static and dynamic parts of the model that we saw at the beginning of Chapter 6.

Our second inclination (on rubbing bleary eyes and staring again at the diagram) was … wait … that it should actually be the reverse—it should be Hotel and not HotelCollection (because it was providing hotel attributes)!

In a situation such as this, where you find yourself switching back and forth between two options, consider strongly the possibility that there should in fact be a third class involved. In this case, the confusion arises because Hotel is schizophrenic and doesn’t know if it’s one hotel or several hotels (and because there’s a disconnect between the static and dynamic models). So the solution is to introduce a new class that can be both Hotels and HotelCollections. Let’s call the new class MultipleHotelCluster.

“Hotel” on this diagram really means “all the hotels that are too close together to resolve as individual hotels.” So our new MultipleHotelCluster class is a HotelCollection within which all the hotels are, for practical purposes, at the current map scale, geographically coincident.

If you’re an avid design-pattern spotter, you’ll recognize that we’ve just described the Composite pattern. We’ll revisit this part of the design later in the chapter, when we get into the nitty-gritty aspects of the design. We just mention it now because it figures in the robustness diagram (see Figure 7-3) and has some bearing on the analysis thought process at this stage.

Figure 7-3: Updated “Display Rollover Information” robustness diagram

Figure 7-3 shows the updated version of the robustness diagram. It’s very nearly complete, but there are still one or two improvements that can be made.

Step 3: Fine-tuning the Diagram

Let’s take a look at the Entity objects in Figure 7-3 and give those a good going-over next.

1. The diagram now shows MultipleHotelCluster, but it also needs to show Hotel.

2. Hotel attributes come from a Hotel and are displayed in a Hotel Info Popup when a single hotel is clicked (via the “On Click single hotel” connector). Ergo, Get Hotel Attributes needs to be connected to … Hotel (not to MultipleHotelCluster).

3. The class MultipleHotelCluster has no hotel attributes; it’s merely a container for Hotels that happen to be geographically close together. So it makes no sense to have a connection between MultipleHotelCluster and Get Hotel Attributes. This is because Get Hotel Attributes is going to be a method on the Hotel class. Therefore, the arrow at the bottom right of the diagram needs to be deleted, an entity (Hotel) needs to be added to the right of MultipleHotelCluster, and that entity (Hotel) needs to be connected to Get Hotel Attributes.

Now on to the Boundary objects:

1. Currently the diagram shows two boundaries: Hotel Info Popup and Map Viewer. We could do with a third: Map Tip Window. This third Boundary object is really just for symmetry (and, of course, to make the robustness diagram match the use case text). If there’s a boundary for the pop-up, why not also have one for the map tip? (Recall from the UI description that pop-ups and map tips are very similar beasts, so they should be modeled consistently.) So …

2. We need to add a boundary called Map Tip Window toward the upper left of the diagram.

3. We then disconnect “Show the tip window” from Map Viewer and connect it to Map Tip Window. The result of the last few steps is that on the right side of the diagram, Show Hotel Info Popup is connected to Hotel Info Popup, and on the left side of the diagram, “Show the tip window” is connected to Map Tip Window.

4. Finally, on refactoring this diagram, we noticed that the “Show List of hotels” controller isn’t actually needed at all. This is because it’s redundant with the “Get Hotel name(s)” controller. So “Show List of hotels” can safely be removed altogether.

Figure 7-4 shows the final version of the robustness diagram, before we move on to the sequence diagram.

Figure 7-4: Final “decluttered” and disambiguated “Display Rollover Information” robustness diagram

As you can see, a lot has changed between this version of the diagram and the original version shown in Is Robustness Diagramming an Art or a Science?”

Remember that with robustness diagrams, we’re producing a conceptual design. This gives us a great opportunity to spend a little time getting the conceptual (high-level) design right, creating a solid foundation from which to design the implementation.

Code-level refactoring is often the result of code that has been written to the wrong conceptual design. For example, we never imagined the need for a Hotel Cluster object, so we just jammed the behavior for multiple hotels into the Hotel object, giving Hotel a bad case of schizophrenia because it didn’t know if it was one Hotel or several.

However, even for people who initially find robustness diagrams difficult to get right, it really does become much easier with practice. Doug’s experience from teaching hundreds of JumpStart workshops is that it takes working through about a half-dozen robustness diagrams to get good at it. Your first one might take an hour, but by the time you’ve worked through five or six of them and gotten the hang of it, you’ll be able to knock them out in about 5 minutes. It’s a lot like riding a bicycle: once you figure it out, it’s really quite easy. The benefit you gain from this perseverance is the ability to design systems that are highly modular and easy to maintain, and that stand a much better chance of being correct.

Modeling Tip 

Use the highlighter test to check if your robustness diagram is right. Remember that the robustness diagram is an “object picture” of your use case. Check the robustness diagram against the use case text by using a highlighter pen to go through your use case one sentence at a time, and highlight the corresponding objects and links on the robustness diagram. If they don’t match, make them match. This will often mean rewriting (disambiguating) the use case.

“Display Rollover Information” Sequence Diagram Revisited

Now that we’ve corrected the robustness diagram, we can safely move on to the next stage: the sequence diagram (the original version is shown back in Chapter 3):

1. Use the boundaries, actors, and entities.^[4.]

2. Copy the use case text.

3. Work through the controllers one at a time.

What we actually find is that as we start to produce the sequence diagram, new design concerns become apparent. In general, you’ll often find that these concerns feed back into the robustness diagram, and sometimes they even go as far back as the use case and domain model.

For this particular example, we’ll see the sequence diagram develop over several short, fast passes. We’ll see the diagram split into three separate diagrams. Then, as our understanding of the design improves, two of these diagrams get merged. Finally, we converge on a design (with the active involvement of the programmer responsible for this area of the code). This is not dissimilar to the way that a programmer refactors code, evolving his way to a cleaner design. However, the approach we’re taking is faster because there are no unit tests to write and rewrite, and no source code to slow us down (yet).

Let’s walk through this process in more detail.

“Display Rollover Information” Sequence Diagram: The First Refactoring

As we began refactoring the sequence diagram from Figure 6-9, we realized that the use case is really describing three different events:

• What happens when the user hovers the mouse over a Hotel icon

• What happens when the user clicks a Hotel icon

• What happens when the user clicks a Multiple Hotels icon

MODELING QUESTION: IS THIS ONE USE CASE OR THREE?

The fact that this use case is describing three different events could theoretically justify creating three separate use cases. However, this really is just a single use case.

The use case is the conceptual activity that the user is doing: she’s rolling the mouse over the icons on the map and the system is displaying details about what she rolls over. Whether it’s one hotel or a cluster that she happens to roll over doesn’t make it a different use case—it’s just different behavior within the “Display Rollover Information” use case.

For the purposes of displaying the sequence diagram in this book, we’ve divided it into three. Normally, however, the basic course and the alternate courses should all be shown on the same sequence diagram, with the message arrows lined up opposite the text (see 7-7 show the three parts of the sequence diagram.

Figure 7-5: “Display Rollover Information,” zeroed in on the “Move mouse over a Hotel icon” event

Figure 7-6: “Display Rollover Information,” zeroed in on the “Click on a single Hotel icon” event

Figure 7-7: “Display Rollover Information,” zeroed in on the “Click on Multiple Hotels icon” event

In the use case (shown at the left of Figure 7-5), the text that’s relevant to the sequence diagram is as follows:

The user positions the mouse cursor over a hotel icon in the map display. Mouse icon changes to “selection hand” and the hotel icon changes to “selected hotel.” The system displays a map tip, displaying the hotel name.

In the use case text (shown in the text at the left of Figure 7-6), the text that pertains to the sequence diagram is as follows:

The user may click on the hotel icon to display a hotel info popup that displays the hotel’s attributes for all of the fields in the hotel popup fields collection … [and so on to the end of the basic course].

In the use case text (shown in the text at the left of Figure 7-7), the text that pertains to the sequence diagram is the alternate course:

Multiple hotels found at click coordinates: System constructs a list of the selected hotels’ names and displays them in a popup.

Notice that this part of the sequence diagram uses MultipleHotelCluster instead of Hotel, as it’s dealing with multiple hotels.

“Display Rollover Information” Sequence Diagram: The Second Refactoring

Having separated out the diagrams, we began to think some more about the Composite design pattern (discussed earlier in this chapter; see the sidebar titled “Object Discovery in Action: Introducing MultipleHotelCluster”) and how this could help to simplify the design. The essence of the Composite design pattern is that to the client code, it makes no difference whether it’s dealing with a single object or a cluster of similar objects. For our example, this translates to a single Hotel or a MultipleHotelCluster.

To describe the pattern in UML parlance

AOI has a HotelCollection, which has MultipleHotelClusters and Hotels. MultipleHotelCluster is a HotelCollection, and it has Hotels.

(Note that we can read has as a UML Aggregation relationship, and is as a UML Generalization relationship).

We began to put this into a class diagram to clarify the design in our minds. The initial version that we produced is shown in Figure 7-8.

Figure 7-8: Refactoring Hotel and HotelCollection into an (almost) Composite design pattern

Strictly speaking, this isn’t a 100% conformant Composite pattern. If it was, then the “children” relationship between Hotel and MultipleHotelCluster would instead be pointing from MultipleHotelCluster back to HotelCollection. This would allow HotelCollection (theoretically at least) to be composed of an infinite number of nested Hotels and MultipleHotelClusters. For our purposes, however, this is overkill; we want the collection to be only one level deep. That is, a HotelCollection may be a single Hotel or a cluster of Hotels, but not a cluster of clusters. So for our purposes, it makes sense for “children” to point directly to Hotel (effectively closing off the loop and preventing recursion) rather than back to HotelCollection.

As we produced this class diagram, we began to think about the class names we were using. If HotelCollection was a single Hotel, then it didn’t seem to make sense for it to be a “collection” as such. We also agreed that MultipleHotelCluster was a bit of a mouthful, so we began to refactor the class names. What we ended up with is in Figure 7-9. As you can see, HotelCollection became HotelComposite (in tribute to the Composite pattern that we borrowed and then mangled shamelessly for our own purposes). And MultipleHotelCluster became simply HotelCluster (which takes fewer brain cycles to read but conveys basically the same information).

Figure 7-9: Our semi-Composite design with some of the classes renamed

Although we don’t normally split hairs about such things, note that AOI and HotelComposite have an aggregation relationship (meaning AOI contains some HotelComposites), whereas HotelCluster and Hotel have a much stronger composition relationship (meaning essentially that a HotelCluster consists purely of Hotels. Building a HotelCluster without Hotels would be like building a brick house without bricks).

Now that we have our HotelComposite all sorted out, we can examine how it affects the rest of the design. Recall from earlier that we had modeled the use case as three separate sequence diagrams (shown in Figures 7-5, 7-6, and 7-7) to cover each of the scenarios. Two of these were “Click on a single Hotel icon” and “Click on Multiple Hotels icon.” But of course, the whole point of adopting the Composite pattern is that the design doesn’t need to differentiate between single and multiple hotels. So we should, in that case, be able to combine Figures 7-6 and 7-7.

This will leave us with two sequence diagram “fragments”: Click HotelComposite and Mouse Over HotelComposite. Figure 7-10 shows the “final” refactored sequence diagram for this use case, including the Mouse Over event and Click Hotel/HotelComposite events.

Figure 7-10: “Display Rollover Information,” using the Composite pattern to combine the single hotel and multiple hotel scenarios

Notice that the use case text has been split up so that the text lines up more or less with the messages in the diagram (the exception to this is the “No hotels are found at click coordinates” alternate course, which isn’t shown).

Figure 7-10 also introduces a new controller object, AllHotelsOnMap. This is simply an instance of the AOI class.

To sum up, this is the essence of model refactoring: iterating through various design possibilities until we zero in on the design that works for everyone involved. We believe (and we hope you’ll agree) that this is far more efficient (and leads to a higher quality product) than coding “the simplest thing that could possibly work” and refactoring the design into shape later. We believe that a concerted effort to get the design right the first time is worth its weight (figuratively speaking) in gold.

Class Diagram

Cast your mind back to the reverse-engineered diagram in Figure 6-11. Let’s compare that with our new class diagram in Figure 7-11, the product of our analysis-level and design-level refactorings so far.

Figure 7-11: Refactored class diagram for the mapplet prototype

As you can see, Figure 7-11 still needs work. We deliberately show it here in an unfinished state, because we’re going to delve into the source code for the prototype and refactor parts of it. This will be fed back into the class diagram, so there would be little point in trying to perfect the diagram at this stage.

MODELING VS. DOING THE SIMPLEST THING THAT COULD POSSIBLY WORK AND THEN REFACTORING

It’s worth pausing for a moment here to reflect on what we’ve just done in our model. We started out with a very simple model: a Hotel class that was hiding a dual personality by doing the work of both a single hotel and a cluster of hotels. This was in fact, exactly “the simplest thing that could possibly work,” because the prototype release was actually coded that way, and it ran. But it was far from the best design. We then went through a number of refactorings of the design and saw Hotel become Hotels, MultipleHotelCluster, and HotelCluster, before finally arriving at the Composite pattern.

Could these refactorings have been done in code, without modeling? Sure they could have. But they would have carried the additional overhead of writing and running unit tests every step of the way. We believe it’s much more efficient to learn how to manipulate the design using UML. But here’s the lesson we hope this example teaches you: myopically Doing The Simplest Thing That Could Possibly Work (DTSTTCPW) and “just coding what we need today” will almost always generate the need for massive amounts of refactoring. This “continual rework because we didn’t get it right the first time syndrome” (i.e., Constant Refactoring After Programming) can be avoided by using a little bit of foresight. In this case, one (highly questionable) agile practice (DTSTTCPW) generates the need for another compensatory agile practice (refactoring).We discuss this sort of thing at great length in Extreme Programming Refactored: The Case Against XP.

Source Code for the Refactored Design

Here’s the source code for the refactored design using our adapted version of the Composite pattern.

Source Code for HotelComposite

We’ll start with the abstract class HotelComposite (refer to Figure 7-11):

 public abstract class HotelComposite  {      protected int X;      protected int Y;      public HotelComposite (int aX, int aY)      {          X = aX;          Y = aY;      }      public int GetX ()      {          return X;      }      public int GetY ()      {          return Y;      }      public abstract string GetHint ();      public abstract string GetTitle ();      public abstract string GetHighestPriceBand ();      public abstract int GetHotelCount ();      public abstract string GetRolloverInfo (                                    int ImageIndex,                                    LookupCollection Amenities,                                    string refURL);      public abstract string GetPriceBand ();  } 

The X and Y member variables are simply the HotelComposite’s X and Y coordinates on the map.

Some of the abstract methods in HotelComposite didn’t appear in Figures 7-10 and 7-11, because the methods are derived from the other use cases (which we’ve left out for brevity). But there’s also a method, GetHotelCount(), which we forgot to include in the sequence diagrams and class diagram. That sort of thing is to be expected: the code verifies the design equally as much as the design verifies the code. As soon as the programmer really gets down to the source code level, he’ll discover parts of the design that need to be changed. If he’s followed a stringent design process, these changes should be minimal, but it’s important to recognize that they do still occur and to bring the design diagrams back in sync as quickly as possible.

As this code was written, it quickly became obvious that our GetDisplayName() method wasn’t quite right. For the purposes of constructing the web page, simply showing a display name wasn’t sufficient. Instead, we need a method that returns the entire rollover information. And so the replacement for GetDisplayName(), at least in spirit, is GetRolloverInfo().

GetRolloverInfo() actually returns a small snippet of HTML that gets added to the web page. We’ll focus on GetRolloverInfo() as we explore the remainder of the source code, because this gives us the essence of the Composite pattern that is at the heart of our design refactoring efforts in this chapter.

Source Code for Hotel

Here’s the concrete Hotel class, focusing on the GetRolloverInfo() method (again, refer back to Figure 7-11):

 public class Hotel : HotelComposite  {      private string id;      private string name;      private string address1;      private string address2;      private string city;      private string state;      private string zip;      private string phone;      private string amenities;      private string starRating;      private string brand;      private string priceBand;      private string links;      public Hotel (string aID,          int aX,          int aY,          string aName,          string aAddress1,          string aAddress2,          string aCity,          string aState,          string aZip,          string aPhone,          string aStarRating,          string aAmenities,          string aBrand,          string aPriceBand,          string aLinks): base (aX, aY)      {          id = aID;          name = aName;          address1 = aAddress1;          address2 = aAddress2  ;          city = aCity;          state = aState;          zip = aZip;          phone = aPhone;          amenities = aAmenities;          starRating = aStarRating;          brand = aBrand;          priceBand = aPriceBand;          links = aLinks;      }      public string GetName ()      {          return name;      }      public override string GetRolloverInfo (int ImageIndex,                                  LookupCollection Amenities,                                  string refURL)      {          return DisplayWindowText (Amenities, refURL);      }      private string DisplayWindowText (LookupCollection Amenities,                                       string refURL)      {          string s;          if (links.IndexOf ("O") < 0)              s = name;          else          {              string webPage = String.Format (refURL, id, "O");              s = "<A HREF=\"" +                  webPage +                  "\" target=blank>" +                  name +                  "</A><BR>";          }          bool added = false;          LookupItem item;          string delimStr = " ";          char [] delimiter = delimStr.ToCharArray();          string [] a = amenities.Split (delimiter);          foreach (string st in a)          {              if (st.Length > 0)              {                  item = Amenities.Find (st);                  if (item != null)                      if (item.info != null)                      {                          added = true;                          s = s + "<IMG src='/books/4/469/1/html/2/" +                                  item.info +                                  "' ALT='" +                                  item.val +                                  "'/>";                      }              }       }          if (added)              s = s + "<BR>";          string Address = address1;          if (address2.Length > 0)              Address = Address + "<BR>" + address2;          Address = Address + "<BR>";          if (city.Length > 0)              Address = Address + city;          if (state.Length > 0)              Address = Address + ", " + state;          if (zip.Length > 0)              Address = Address + ". " + zip;          s = s + Address;          if (links.IndexOf ("B") >= 0)          {              string webPage = String.Format (refURL, id, "B");              s = s + "<BR><A HREF=\"" +                      webPage +                      "\" target=blank>View hotel brochure</A>";          }          string wp = String.Format (refURL, id, "R");          s = s + "<BR><A HREF=\"" +                  wp +                  "\" target=blank>Check Rates</A>";          return s;      }      public override int GetHotelCount ()      {          return 1;      }  } 

Notice the GetHotelCount() method, which we discussed earlier. You can see here how it works in the context of the Composite pattern. GetHotelCount() is defined as an abstract method in HotelComposite. In Hotel, it simply returns 1, as Hotel is obviously just the one hotel itself. However, as we’ll see in a moment, HotelCluster will instead return a count of all its Hotels.

Taking a look at the source code for Hotel, one question that springs to mind is why the DisplayWindowText() method needs to be a separate method from the one-line GetRolloverInfo(). In fact, we’d better not let the refactoring police get too close to this code, as they would immediately want to break the fairly large DisplayWindowText() into a series of smaller, more fine-grained methods (and rightly so!). For example, the code (halfway through the method) that concatenates various strings to make an Address is a prime contender for being extracted into a separate method, somewhat like this:

 public string ConcatAddress ()  {     string Address = address1;     if (address2.Length > 0)         Address = Address + "<BR>" + address2 ;         Address = Address + "<BR>";         if (city.Length > 0)             Address = Address + city;         if (state.Length > 0)             Address = Address + ", " + state;         if (zip.Length > 0)             Address = Address + ". " + zip;         return Address;  } 

Also, the DisplayWindowText() method starts very cryptically:

 string s;  if (links.IndexOf ("O") < 0)      s = name;  else  {      string webPage = String.Format (refURL, id, "O");      s = "<A HREF=\"" +          webPage +          "\" target=blank>" +          name +          "</A><BR>";  } 

Someone trying to find her way around this code would likely be scratching her head, wondering about the significance of the links string containing the letter “O,” and to whom the letter “O” might be important. Let’s briefly explore what the “O” check means. Then we can decide whether it’s worth refactoring and feeding back into the model. At the very least, we could add a comment to the code, so that the next person who arrives at this class won’t need to do the same rooting around to work out what it’s doing.

There are three fields in the hotel database that have a Y/N value: Brochure, Overview, and Rate. A Visual Basic DLL running on the server queries the database and returns a string of which info pages are available for each hotel. These are returned to the .NET application in the form of a string (e.g., BOR means “Brochure, Overview, and Rate,” and BR means “Brochure and Rate”). So by checking for the letter “O,” the code is actually checking whether this Hotel object has an Overview available for it.

It’s worth reiterating the point that the team was prototyping at this stage, so it’s not surprising that this implementation detail wasn’t picked up in the up-front design. However, we’re now at the stage where we’re bringing the code and the design into sync (so that we can do a proper design for the next release), so we need to decide whether it’s worth refactoring this now, or simply add a note to the code (or to our UML model) so that when we return to the model for the next iteration, we’ll know to address this area. Because we happen to be staring at the code at the moment, let’s examine what would be needed to tidy up this area of the code and make it more self-explanatory.

Coding by intention is an important agile principle—that is, write the code in terms of what you want it to do, not how you want it to do it. Following this principle results in code that is self-explanatory and needs fewer code comments. For our mysterious letter “O” example, we could replace the check with a function call whose name actually states what the check is doing, for example:

 string s;     if ( !HasOverview() )     {      s = name;  else  {        // . . . 

The links variable is a member of the Hotel class, so we don’t need to pass it in to our new HasOverview() method. Our new method is as follows:

 private bool HasOverview ()  {      return links.IndexOf ("O") >= 0;  } 

Notice that we’ve reversed the links.IndexOf check. Previously, it was returning true if links did not contain the letter “O.” Now it returns true if there’s a positive match, which in our minds is a little less brain-warping. You can also see the result of this in the code that calls the new method, if ( !HasOverview() ), in other words, if this Hotel does not have an Overview …

We would also add similar methods for HasBrochure and HasRate as and when we discover that we need them. We can now see much more easily what the code is doing (i.e., checking that this Hotel has an Overview), without needing to add code comments. In the previous version, this just wouldn’t have been obvious from looking at the code.

This early prototyping is useful because it allows us to feed these methods back into the UML diagrams, having proven in the code that they’re going to be needed. In fact, what often happens is that, as we feed back into the UML, we see an overall pattern taking shape, which we might otherwise have missed. This allows us to further streamline the design and feed the improvements back into the code, and so on.

Of course, this refactoring still leaves us with quite a lengthy DisplayWindowText() method overall. So perhaps this whole section could be moved into a separate method, something like this:

 private string CreateOverviewLink ()  {     if ( !HasOverview() )        {         return name;        }     string webPage = String.Format (refURL, id, "O");     return "<A HREF=\"" +            webPage +            "\" target=blank>" +            name +            "</A><BR>";  } 

Anyway, there’s quite a bit more refactoring that could be done to this code (mainly involving breaking up the DisplayWindowText() method into smaller methods), but we’ll leave that until release 2. At this stage, we’re concentrating on enabling the next effort, not actually implementing the next effort, so it’s important to not get too carried away changing things around.

Source Code for HotelCluster

Finally, here’s the prototype implementation of the GetRolloverInfo() and GetHotelCount() methods in HotelCluster:

 public class HotelCluster : HotelComposite  {      private ArrayList hotels;      public HotelCluster (int aX, int aY): base (aX, aY)      {         hotels = new ArrayList ();      }   public override string GetRolloverInfo (int ImageIndex,                               LookupCollection Amenities,                               string refURL)   {       string RolloverInfo = "";       Hotel h;       for (int HotelCount = 1;            HotelCount <= hotels.Count;            HotelCount++)       {           h = (Hotel) hotels [HotelCount-1];           RolloverInfo = RolloverInfo +                  "<A HREF='javascript:updateTheLayer " +                  "(document.WebForm.hotel_" +                  ImageIndex.ToString () +                  ".getAttribute (\"Title_" +                  HotelCount.ToString ()                  + "\"), document.WebForm.hotel_"                  + ImageIndex.ToString ()                  + ".getAttribute (\"RolloverInfo_"                  + HotelCount.ToString ()                  + "\"))'>"                  + h.getName ()                  + "</A><BR>";       }       return RolloverInfo;   }      public override int GetHotelCount ()   {       return hotels.Count;   }  } 

Not much to say here, except that, again, this code will be a prime candidate for refactoring when we get to the next release. For example, the block of code in the middle of the method that puts RolloverInfo together could be moved into a separate method called BuildRolloverInfo(). Again, this new method would be fed back into the appropriate UML diagram.

Feeding Our Changes Back into the Model

Because the purpose of this exercise is to bring the source code and the model into sync, we ought to feed these changes back into the class diagram (see Figure 7-13). If the behavior of our classes had significantly changed, we’d also want to update the sequence diagrams. But luckily, the relatively minor changes that we’ve made are mostly structural in nature.

Figure 7-13: Refactored class diagram

Let’s start with HotelComposite. To jog your memory, Figure 7-12 shows a “detail” of HotelComposite and its two subclasses from the class diagram.

Figure 7-12: HotelComposite and its subclasses before the code-level refactoring

GetDisplayName() didn’t actually make it as far as the code. Instead, two new methods were added at coding time: GetTitle() and GetRolloverInfo(). This was one of those cases where, at the time, it seemed like it would be sensible to have a GetDisplayName() method. But in reality, something else was needed, so we got rid of it. Never be sentimental with unused code. If it isn’t used, get rid of it!^[5.]

There’s also a whole bunch of abstract methods in HotelComposite, which we didn’t show in the original class diagram (because they’re defined by the use cases that we left out for brevity’s sake). These are GetHint(), GetHighestPriceBand(), and GetPriceBand().

And finally, if you recall, GetHotelCount() was missing from the original class diagram but was discovered during coding. This seems like a good time to feed it back into the class diagram.

Next, let’s move on to the Hotel class (a concrete subclass of HotelComposite). The original version of Hotel (just prior to coding) is also shown in Figure 7-12. An attribute that was added to Hotel is Name (with an associated GetName() accessor method). But hang on a second—this seems remarkably similar to the abstract method GetDisplayName(), which was removed from the parent HotelComposite. The reason GetDisplayName() was removed was because it was replaced with GetRolloverInfo(), which constructs a tool tip including the name. To confirm this, we can see that in Hotel’s implementation of the GetRolloverInfo() method, it uses the Name attribute to construct part of the tool tip. So Name definitely belongs, and it serves a different purpose from the DisplayName attribute, which was removed. Whew—that seems like a lesson in minutiae, but sometimes it’s worth checking these things in case they throw up a more deep-seated design issue (e.g., a misunderstanding might have arisen that resulted in a duplicate attribute).

We also did some refactoring on the Hotel class: we created a new private method, HasOverview(). Plus, we created HasBrochure() and HasRate() (waiting in the wings for when we need them). These are private methods, and they may be a bit too low level to warrant showing on the class diagram, so it’s probably better not to show them.

Finally, let’s move on to HotelCluster. We hinted at some refactorings that could be done on this class, but we didn’t dive into them just yet, instead opting to leave them until release 2, when we’ll need to revisit the code anyway. However, our earlier refactorings (the removal of GetDisplayName() and the addition of the abstract methods that need to be implemented by both Hotel and HotelCluster) do affect HotelCluster.

HOW TIGHTLY SHOULD THE UML BE BOUND TO THE CODE?

Knowing how tightly to bind the code and the design is a burning issue. If we reflect every last detail of the source code in the class diagrams and sequence diagrams, we’d still be finishing off the same diagrams in several years’ time, probably after the customer has gone out of business. Conversely, if we draw the diagrams at such an abstract level that they’re largely meaningless, then that’s not much use either. So finding the “sweet spot” (and knowing it when you see it) is an important design skill.

Similarly, every change we make to our design diagrams needs to be justifiable. Simply saying,“We must do this because we’re following a design process” isn’t sufficient. It’s important to be able to say,“We’re making this change because it will help to make XYZ clearer later on.”

In the Hotel example, it’s doubtful that adding the private methods (GetOverview(), etc.) to the class diagram would add value, because we’d simply be replicating what’s in the source code (and binding the class diagram too tightly to the code). This would mean that there’s more diagram to maintain when we change the code.

In addition to generating code and importing code, the Enterprise Architect (EA) tool has the option to synchronize the model and source code. For example, say you’ve already generated some source code, but you’ve made subsequent changes to the model. When you generate again, EA will add any new attributes or methods to the existing source code, leaving intact what already exists. This means that developers may work on the source code and then generate additional methods as required from the model, without having their code overwritten or destroyed.

In another scenario, changes may have been made to a source file, but the model has detailed notes and characteristics you don’t want to lose. By synchronizing from the source into the model, additional attributes and methods are imported, but other model elements are left alone. Using the two synchronization methods, it’s simple to keep source code and model elements up to date and synchronized.

So that about wraps up our changes between release 1 and release 2. The updated class diagram is shown in Figure 7-13.

As you can see, HotelCluster seems fairly empty now. In fact, it does implement all the methods that are shown as abstract in HotelComposite. We just haven’t shown them on the diagram because it seems kind of obvious: HotelCluster is concrete, therefore any abstract methods in the parent class would have to be included. It’s worth taking this sort of shortcut if you’re absolutely sure that it won’t cause confusion in the project. When in doubt, put the detail in!

As for the other classes on the diagram, we’ve updated some of the method names. MapViewer now looks much more streamlined, with only one public method. The other methods, it turned out, just weren’t needed in the implementation; to fit into the Web Forms framework, just Page_Load was needed. So we’ve updated the diagram to show this. This will prove useful when we’re designing the next release, because we have it on good authority (working source code!) that the MapViewer Boundary object works this way.

We’ve also updated the other classes’ relationships to show the multiplicity. Earlier, this wasn’t really necessary (plus we would have been guessing), but now that we have a solid prototype to refer to, we can add this information and be pretty confident that it’s right.

And finally, we’ve updated the relationship between the Map Server and Hotel Server to show this as Composition, because (referring back to the architecture discussion earlier in this chapter) the Hotel Server is really a part of the Map Server.

The design shown in Figure 7-13 is the one that we’ll build upon for release 2, which begins in earnest in the next chapter.

When to Keep the Model and the Code Tightly Interwoven

In Chapter 6, we walked through the source code for release 1 and refactored the code in a couple of places (e.g., we introduced a new AOI class). A natural consequence of this (which occurs in every single project that includes a documented design of some sort) is that the source code and the design model became out of sync.

The Agile Modeling (AM) guidance on the subject is to “update only when it hurts.” That is, update the design diagrams only when you find that working from an out-of-date document is starting to affect the project. Our guidance is different because AM does not propose driving the code from the model (instead, the model is a marginally useful “document” that gets produced and is not maintained). In Agile ICONIX, we drive the code from the model.

This is a hugely important distinction. AM drives the design of the code by refactoring. Agile ICONIX makes a dedicated effort to get the design right the first time, by “modeling like we mean it” (one small use case at a time) and using refactoring when minor cleanup is needed. Note that this doesn’t mean we need to spend months redrawing sequence diagrams after the code has been changed around so that we can have a large stack of useless documentation. It means that we spend a little more time trying to get the sequence diagrams right before coding.

The nature of an agile project is that we’re never really 100% sure where the requirements are going to drift. And so doing a bit of housekeeping on the model so that it can remain useful moving forward is just a good thing to do after each release. (Note that this is also in keeping with the AM core principle that “enabling the next effort is your secondary goal.”)

In the spirit of “traveling light” (another AM core principle^[6.]), it’s not necessary to redraw all the sequence diagrams. But the static model (class diagrams) should be cleaned up, and the classes in the model should match the classes in the code. If you update only when it hurts, you’ll quickly toss out the model because it has become useless and obsolete.

The creation of the new AOI class was a result of revisiting the original design and finding that we’d missed it in the code. In this case, we were updating the code to keep it in sync with the design, which in this instance resulted in better, more tightly factored code. However, in other cases, the design has evolved during the programming iteration, and these changes need to be fed back into the design model.

Note, however, that updating every sequence diagram to reflect the code could end up being a waste of time if the diagrams are never going to be used again. The code has already been written, so to a large extent the sequence diagrams (the detailed, low-level design artifacts) have served their purpose.

To determine which parts of the model need to be updated, we refer to the plan for the next release (assuming the features for the next release increment have been picked already). We use this to determine which parts of the project we’re going to be extending in the next release. These are, of course, the areas that will need up-to-date design documentation.

Some of the other differences between the model and the mapplet code were because we didn’t follow ICONIX Process 100% during coding, which resulted in source code that didn’t precisely fit the design. This isn’t a cardinal sin—at the end of the day, we’re mostly interested in creating finished, working code that fulfills the original requirements. However, we do find that if the code and the design diverge too much, then it becomes more difficult to design the next iteration. If the code and the design are roughly similar, then we can reuse a lot of the design artifacts (the domain model, in particular) and also build on the original set of use cases for the next release.

^[3.]Of course, when we get to the sequence diagram, it will be a different matter. Sequence diagrams are all about low-level design, so at that stage we will want the extra detail on the diagram.

^[4.]For example, using Enterprise Architect, this literally means “drag the boundaries, actors, and entities from the browser window into the sequence diagram.”

^[5.]It should go without saying that all your source code is under version control (including prototype code), so if you later discover that you really did need that method you deleted, it isn’t lost forever.

^[6.]From Scott Ambler’s Agile Modeling: Effective Practices for eXtreme Programming and the Unified Process (Hoboken, NJ: John Wiley & Sons, 2002), p. 29: “Traveling light means that you create just enough models and documentation to get by.”