It's certainly not immediately obvious from looking at the code (which we'll get to very soon), but the rather dysfunctional example in this chapter is intended to be a price calculator that finds specific hotels in a database and then calculates how much it would cost to stay at that hotel for 'n' nights. The price may fluctuate (admittedly not as much as, say, a Forex price feed), but the coders saw fit to add in a live "hotel price streamer." This streamer listens to an external price streaming service, and feeds the results back into the hotel price calculator.

To map things out a little and provide a "big picture," Figure 9-1 shows a UML class diagram with all of the classes and their tangle of relationships. We're sure you've felt that moment of creeping dread when you know that you're about to dive into a particularly nasty area of legacy code, like plunging into a black tunnel, a deep crypt where untold horrors may lurk. It's because of this feeling that we refer to the code in this chapter as the "Temple of Doom."

Figure 9-1. Dare you enter the Temple of Doom?

Over the next few pages we lay out all of the "Temple of Doom" in its inglorious horror. If you see code swimming before your eyes, feel free to skip this section—it's mainly just there for the "shock factor" anyway—as we'll present the relevant snippets of the code as we explore each antipattern.

So duck under your adventurer's fedora hat and take a swig from your authentic stitched-camelhide water bottle, as you enter the Temple of Doom (if you dare), where programmers were sacrificed to Quetzalcoatl, the ancient Mayan god of refactoring.

As you step trepidatiously into the outer crypt of this legacy hellhole, rays of sunlight from the crumbling doorway play across the main class, HotelPriceCalculator. Here's the code for the class:

package com.softwarereality.nottherealmapplet;

import java.math.BigDecimal;
import java.util.*;
import java.util.logging.*;

public class HotelPriceCalculator
    extends HotelCollection

implements PriceListener {

    static Logger logger;
    static final HotelPriceCalculator nyInstance =
          new HotelPriceCalculator(null);
    final PriceStreamer priceStreamer = PriceStreamer.getInstance();
    static List<Hotel> nyHotels;

    static {
        try {
            logger = Logger.getLogger("not.the.real.mapplet");
            PriceStreamer.getInstance().addListener(nyInstance);
            nyHotels = ((HotelDB) OracleManager.getDao(HotelDB.class)).searchFor("New York");
        } catch (Exception e) {
            // fail silently
        }
    }

    public HotelPriceCalculator(Object ... hotelIDs) {
        super(logger);
        try {
            PriceStreamer.getInstance().addListener(this);

            // many lines of code to load up a list of Hotels from the DB:
            Map<Object, Hotel> hotelsByID = new HashMap<Object, Hotel>(hotelIDs.length);
            DAO db = OracleManager.getDao(HotelDB.class);
            for (Object id : hotelIDs) {
                Hotel hotel = (Hotel) db.find(id);
                hotelsByID.put(id, hotel);
            }
            super.setHotels(hotelsByID);

        } catch (Exception e) {
            logger.log(Level.SEVERE, "Error initialising", e);
        }
    }

    public BigDecimal calculatePrice(Hotel hotel, int numNights) {
        Object id = hotel.getId();
        BigDecimal pricePerNight = priceStreamer.quotePrice(id);
        return pricePerNight.multiply(new BigDecimal(numNights));
    }

    // "Just in case" code - why 5 nights? Uncompleted task?
    // Programmer's thought in progress?[49] Bad dream?
    public void priceUpdate(Object id, BigDecimal price) {
        try {
            DAO db = OracleManager.getDao(HotelDB.class);

Hotel hotel = (Hotel) db.find(id);
            calculatePrice(hotel, 5);
        } catch (Exception e) {
            logger.log(Level.SEVERE, "Error updating hotel price", e);
        }
    }
}

Going deeper into the mostly silent crypts, pausing only to wonder if that was the ghostly sound of a centuries-old sacrificial victim, or just the wind blowing through the oppressive tunnel, you peer through the flickering torchlight at HotelPriceCalculator's parent class, HotelCollection:

public class HotelCollection {

    private Logger l;
    private List<Hotel> hotels;
    Map<Object, Hotel> hotelsByID;

    public HotelCollection(Logger l) {
        this.l = l;
    }

    public void setHotels(Map<Object, Hotel> hotelsByID) {
        this.hotelsByID = hotelsByID;
        this.hotels = new ArrayList<Hotel>(hotelsByID.values());
    }
}

You'll see the next class, Hotel, used quite a bit throughout the code. It's rather central to the code's operations, though you wouldn't think so by looking at it. For such an important class, there's not very much to it, but here it is:

public class Hotel {

    private boolean hotelDetailsFound;
    private Object id;

    public Hotel() {}

    public Hotel(Object id) {
        this.id = id;
    }

    public void setId(Object id) { this.id = id; }
    public Object getId() { return id; }

    public static Hotel search(String city, String street) {
        try {
            List<Hotel> foundList = OracleManager.getDao().searchFor(city, street);
            if (foundList.size() == 0) {
                Hotel notFound = new Hotel();
                notFound.hotelDetailsFound = false;

return notFound;
            }
            Hotel found = foundList.get(0);
            found.hotelDetailsFound = true;
            return found;

        } catch (Exception e) {
            e.printStackTrace();
            return null;
        }
    }
}

You're now in the deeper tunnels, ancient crypts that stretch for miles beneath the ground. Everything down here is covered in thick layers of dust, where code has lain undisturbed for centuries. These dense, horrific classes are "services," non-glamorous plumbing code that nonetheless holds everything together and makes the "star performers," well, perform. It's strange, then, how often service classes like these are relegated to the darkest recesses of the project and given the least amount of design consideration—you'll often find a complete lack of object-orientation in them—thus they're extremely difficult to unit-test, even though there's a significant amount of functionality hidden away down here.

Following is the PriceStreamer class, which listens for price updates and notifies any interested listeners when an update arrives:

public final class PriceStreamer {

    private static PriceStreamer instance = null;

    private List<PriceListener> listeners =
                              new ArrayList<PriceListener>();
    private Map<Object, BigDecimal> livePricesByID =
                              new HashMap<Object, BigDecimal>();

    private PriceStreamer() {}

    static public synchronized PriceStreamer getInstance() {
        if (instance == null) {
            instance = new PriceStreamer();
        }
        return instance;
    }

    public void addListener(PriceListener listener) {
        listeners.add(listener);
    }

    /**
     * A price has come streaming in from outside.
     */
    public void update(Object id, BigDecimal price) {
        livePricesByID.put(id, price);

// notify all listeners:
        for (PriceListener l : listeners) {
            l.priceUpdate(id, price);
        }
    }

    public BigDecimal quotePrice(Object id) {
        return livePricesByID.get(id);
    }
}

The OracleManager seems fairly generic from the name; but, as you can see, it's tightly bound to the business domain concept of hotels, with a HotelDB DAO (Data Access Object). Here's the code behind OracleManager:

public class OracleManager {

    static {
        // some trigger-happy code that
        // kick-starts a connection pool
        // with the database... (Omitted from this example)
    }

    // Tightly bound dependency:
    public static HotelDB getDao() throws Exception {
        return new HotelDB();
    }
}

HotelDB itself is intended to be a class that directly queries the database to return Hotel instances. In outline, it's like this (as with OracleManager before it, we've omitted the actual database access code to keep the example under control):

public class HotelDB {

    public Object find(Object id) throws Exception {
        return new Hotel();
    }

    public List<Hotel> searchFor(String ... queryValues) {
        // "Real" database access code
        // would be on or near this line...
        return new ArrayList<Hotel>();
    }
}

And that's the sum total of the code for this chapter. We'll now rewind a bit, and look at the brick walls (or sacrificial temple walls, rather) that we would hit, were we to try adding unit tests to this code.

The most common antipattern by far must surely be the complex constructor. The following code snippet should look familiar, but should also send shivers down your spine:

public HotelPriceCalculator(Object ... hotelIDs) {
    super(logger);
    try {
        PriceStreamer.getInstance().addListener(this);

        // many lines of code to load up a list of Hotels from the DB:
        Map<Object, Hotel> hotelsByID = new HashMap<Object, Hotel>(hotelIDs.length);
        DAO db = OracleManager.getDao(HotelDB.class);
        for (Object id : hotelIDs) {
            Hotel hotel = (Hotel) db.find(id);
            hotelsByID.put(id, hotel);
        }
        super.setHotels(hotelsByID);

    } catch (Exception e) {
        logger.log(Level.SEVERE, "Error initialising", e);
    }
}

Figure 9-2 shows the dependencies that HotelPriceCalculator has already built up, simply from its constructor code.

Figure 9-2. All that baggage, and we haven't even used the new object yet...

This code probably works fine for its originally intended purpose (although there may be issues, both insidious and not-so-subtle, as you'll see in a moment). But it's inherently rigid code: as soon as you want to do something even slightly different from the original purpose—like, hey, add some unit tests—things get very tricky very quickly.

As an example of the problem, let's start to write a JUnit test for the HotelPriceCalculator class:

HotelPriceCalculator priceCalc;
String[] hotelIDs = {"1234"};

@Before
public void setUp() {
    priceCalc = new HotelPriceCalculator(hotelIDs);
}

Before you even get to the test code, there are problems. Simply calling the constructor is going to result in the following:

This all-encompassing initialization work slows the test down, but it also makes the test suite brittle. What if neither the price streaming service nor the database is available? Or someone changes the test data? The data would be external to the build/test environment so probably not under the programmer's total control. This is a particular problem when the unit tests are run as part of the build (which you would generally want); "Service not available" should not count as a build error. Remember that unit tests are meant to be insular: they're testing a unit, a packet of functionality, rather than the outside world. (There are other, end-to-end integration tests for that purpose, but that's a different story.)^[50]

We're sure you've seen constructor code that stretches on for screen after screen, hundreds of lines in length. As you probably noticed, this is not just bad for unit testing—it's also poor design generally: not at all OO. Where's the encapsulation? Where's the splitting out of functionality into separate methods and classes?

You've gone to all the effort of removing the complex initialization code from the constructor. But HotelPriceCalculator isn't just a nice, independent entity; it has an embarrassing parent who's decided to tag along to the party. And who knows what's going on in that parent constructor?

A subtle detail from the previous example was that HotelPriceCalculator extends HotelCollection (a collection of Hotels with added behavior)—see Figure 9-3.

Figure 9-3. Based on true events—only the names have been changed to protect the giddy...

Let's set aside for a moment the dubious design decision that might have led to a price calculator extending a HotelCollection, and peek ahead to the fixed-up HotelPriceCalculator constructor (see Chapter 10):

public HotelPriceCalculator(Object ... hotelIDs) {
    this.hotelIDs = hotelIDs;
}

The parent class, HotelCollection, has a no-arg constructor. In Java, no-arg constructors are called automatically if there are no other constructors available; so if there are subclasses, this sets off a chain reaction moving up the class hierarchy, with each parent constructor called in turn. In the case of HotelPriceCalculator, its parent's HotelCollection constructor will be called first—even though it isn't explicitly invoked in the code. So if there's further complex initialization code up there, then there's even more to potentially go wrong before we have a fully constructed object.

Of course, you can give the same treatment to the superclass as you did for HotelPriceCalculator earlier in this chapter: separate the complex initialization code into separate methods. But then things get complicated, as the order in which the methods are called might be important. Worse, there may be other child classes derived from the parent, which could break if you start messing with the constructor.

This is, of course, why legacy code can be a nightmare to unit-test: it's often impossible to simply change one piece of code, as there's always a hanger-on somewhere that's also affected.

It gets even worse when there's also a superclass above the superclass (and so on up, like fleas teetering on other fleas' backs). It could just be us, but it seems like the number of classes in an inheritance hierarchy is directly proportional to the turnover of programmers in a project. The more coders that have been let loose on a spaghetti plate of code, the taller the class structures will be, like some warped version of the Dubai skyline, or a demonic cluster of towering edifices from one of H. P. Lovecraft's most disturbing nightmares.

If you have the choice, don't even go there. Keep your class structures flat: use inheritance carefully and only where it makes sense. And (as we explore in the next chapter), create a domain model first. Keep it simple, and your code will be both easily unit-tested and easily modified.

One of the most common antipatterns—in fact, a whole nest of the little critters—tends to involve static code (that is, code at the class level rather than the object level... static methods and variables, static initializer blocks, the Singleton pattern...) These can all make unit testing a real nightmare—to the point where many programmers give up, declaring unit testing to be too difficult, more trouble than it's worth.

The static initializer block, or "static hair-trigger," is our favorite example of boneheadedness. In Java, the static initializer block is run automatically when a class is first loaded—this means when the class is first referenced, wherever (or by whomever) it's referenced. Here's an example from HotelPriceCalculator—the initializer block is the bit between the static { and } lines:

static Logger logger;
static final HotelPriceCalculator nyInstance =
    new HotelPriceCalculator(null);
final PriceStreamer priceStreamer = PriceStreamer.getInstance();
static List<Hotel> nyHotels;

static {
    try {
        logger = Logger.getLogger("not.the.real.mapplet");
        PriceStreamer.getInstance().addListener(nyInstance);
        nyHotels = ((HotelDB) OracleManager.getDao(HotelDB.class)).searchFor("New York");
    } catch (Exception e) {
        // fail silently
    }
}

So far, so scary. But let's start to write a unit test for HotelPriceCalculator. Remembering to keep our test-suite self-contained and quick-to-run by avoiding external calls and database connections, we write the following code:

public class HotelPriceCalculatorTest {

    HotelPriceCalculator calc;

    @Before
    public void setUp() throws Exception {
        Object[] hotelIDs = new Object[] {"123", "456"};
        calc = new HotelPriceCalculator(hotelIDs);
    }

    @After
    public void tearDown() throws Exception {
        calc = null;
    }

Before we even get as far as the first test method, there's a problem. Simply blinking or glancing at HotelPriceCalculator would cause the initializer code to run, creating external connections, loading database data, and so on. It causes something of a chain reaction: the HotelPriceCalculator static block refers to OracleManager, whose own static block is as follows (gory details omitted):

static {
    // some trigger-happy code that

// kick-starts a connection pool
    // with the database...
}

This code in turn might reference another class, whose own complex static initialization code is thus triggered... and so on. It is, really, an even more vicious version of the complex constructor, and it massively limits the number of ways in which the class can be tested.

Figure 9-4 shows the chain reaction in graphical form. And all of that just because we wanted to get a copy of HotelPriceCalculator and test its price-calculating algorithm.

Figure 9-4. A chain reaction (static method calls are shown in italics). And all we wanted to do was call calc.calculatePrice()...

This antipattern is also bad form, stability-wise. Note the "fail silently" comment in the code fragment. That isn't fiction—we've seen examples of that many times in real code. But it also brings home a real problem with static initializer blocks: they are only ever run once, when the class is first loaded. If the initialization fails at run-time, the entire class is effectively "out of action" for the rest of the time the program runs. It would need a restart to get the class working again, even if the problem was just a momentary network glitch.

In Chapter 10 we present a better way of achieving what static initializer blocks tend to be trying to achieve, in the section "Avoid Using Static Initializer Blocks."

It's notable that Scala, a modern OO-functional hybrid that compiles to Java bytecode, has done away with static methods and members altogether. Instead, the closest it gets to static types is letting you define an entire class as a singleton. Scala is known for its pragmatic "take" on language design, effectively rebooting Java and throwing out all the bad bits. So it's significant that static members were among the parts that Martin Odersky, Scala's creator, chose to abandon.

Meanwhile, our HotelPriceCalculator has some static members:

static final HotelPriceCalculator nyInstance =
    new HotelPriceCalculator(null);
static List<Hotel> nyHotels;

(logger was also static, but we turned it into an object variable in the previous section.)

But why, you may ask, are static members such a problem when you're unit testing? Unit tests operate best when they're small and well-defined: lots of little test methods creating and destroying fixtures as needed, each test "proving" that one aspect of the implementation works as expected. For this to work well, the tests need the code under test to be self-contained, or encapsulated. Static members, if nothing else, break encapsulation across objects. You can see an example of this in the "Static Just Means Class-Level" sidebar earlier in this chapter.

Your small, self-contained unit tests also rely on being predictable: that is, each object under test beginning with the exact same state every time the test is run. However, if one of the variables under test happens to be static, then another test in the suite may get to it first and give it a different value, making the other test seem to break randomly.

Sure, you can get around this by remembering to set each static member with a specific value in the test's setup() method, but why leave it to chance? You end up setting different values to satisfy different tests, whereas the production system will be far less predictable, with different modules and threads going at it to restore the static member to a value it expects. This creates a situation where a passing test can't be trusted to mean "this code will also work in a production system."

Here is a summary of the problems with static members:

For an example of how static members can be unpredictable, consider a unit test that passes with a "green bar." However, the same code may fail in production because some other class has set the static member to a different value.

To picture the problem, imagine a kitchen with 12 chefs. Over in one corner, a broth is boiling away in a shared saucepan. Chef 1 walks up and adds a dash of Tabasco sauce as he plans to pour the broth over his platter of devilled spare ribs, and returns satisfied to the meat station. Chef 2 wants the saucepan for his authentic Vietnamese eggplant, so he pours in a quart of mackerel sauce. Chef 3, whistling as he works, tips the contents away and replaces it with a pint of beef stock. Chef 4 turns the heat down because he wants the broth to be ready in sync with his entrée. Chef 5 turns up the heat because he needs the contents within the next 60 seconds. Chef 6 adds a turnip. And so on.

It's anyone's guess what will eventually come out of the saucepan. It's like Schrödinger's cat in 12 dimensions. And yet, for all of that, imagine if we ran a unit test for each chef. The tests would be along these lines:

All 12 tests, run individually and with pitch-perfect fixture setup, would pass just fine. But in the kitchen on a Saturday night, chaos ensues and the state of the finished dish is entirely non-deterministic. We return to this issue in Chapter 10, with a proposed solution.

The next antipattern continues the static theme, and should be familiar to just about every OO programmer on the planet.

Here's an example of a Singleton (capital S)^[51] ... pantomime fans should prepare to boo and hiss now:

public class PriceStreamer {
    private PriceStreamer instance = null;
    private PriceStreamer() {}
    static public synchronized PriceStreamer getInstance() {
        if (instance==null) {
            instance = new PriceStreamer();
        }
        return instance;
    }
}

This is possibly one of the most misused patterns in the history of both pantomimes and software design. Exactly why it's so popular escapes us. Perhaps it's because it's such a quick pattern to implement; yet it's rolled out automatically each time a "service" or "manager" type of class is needed. The role of a service or manager class is generally to be a central point of access for some function or other; but rarely does a service or manager genuinely need to be the only instance of its type in the whole system. There's a big difference between "need an object" and "must be the only object of this type in the whole system." In the first case, for the code's purposes it needs an instance of the class, so it creates one—fine and dandy. In the second case, the program would fail in some way if there were more than one instance. However, the second case is needed far less often than you might think.

When you're unit testing, you really want to just be able to create a throwaway instance of the object under test, test it, and then throw it away. The Singleton pattern makes this simple requirement impossible. To see why the Singleton is anathema to unit tests, let's look at an example:

public class PriceStreamerTest {

    private PriceStreamer streamer;

    @Before
    public void setUp() throws Exception {
        streamer = PriceStreamer.getInstance();
    }

    @After
    public void tearDown() throws Exception {
        streamer = null;
    }
}

This should be a familiar pattern by now: we're setting up a test fixture—the PriceStreamer instance in this example. We can't get at the constructor because it's private, so we're forced to call the static PriceStreamer.getInstance() method. Then in tearDown(), which JUnit runs immediately after every test method, we clean up after the test. Unfortunately, simply setting our local copy of PriceStreamer to null doesn't achieve anything, as the instance still exists as a class member inside PriceStreamer itself. So the test is already pretty broken, before we've added any test code.

Let's go ahead and add a couple of test methods anyway, to see what happens.

@Test
public void quotePrice_NoPriceYet() {
    BigDecimal noPriceYet = streamer.quotePrice("123");
    assertNull(noPriceYet);
}

@Test
public void quotePrice_PriceAdded() {
    BigDecimal price = new BigDecimal(200.0);
    streamer.update("123", price);
    BigDecimal quote = streamer.quotePrice("123");
    assertEquals(price, quote);
}

The first test checks that if no price has yet been sent to the PriceStreamer, then we get null if we ask for a quote. Then the second test adds a price (simulating an external price update being received), and attempts to get a quote. If we run this, we get a nice green bar... all good. However, something strange is going on behind the scenes. The first test (NoPriceYet) makes the assumption that we're starting with a clean test fixture—that is, an empty PriceStreamer with no tests added. Figure 9-5 shows what happens if these two test methods were to run in a different order.

Figure 9-5. The first test spoils the fixture for the second test.

With PriceAdded being run first, we suddenly get an inexplicable test failure. This is simply because the same instance of PriceStreamer is being used for both tests. quotePrice_PriceAdded() adds a price to PriceStreamer, and then quotePrice_NoPriceYet() fails because it expects PriceStreamer to be empty.

Suddenly, we realize how fragile the test suite is, when simply rearranging the order that the tests are run can make them fail!

Given that you do sometimes want to ensure that a class is "single-instance only" when you're not unit testing, we present some better alternatives to the Singleton design pattern in Chapter 10.

Often you'll see some code casually create (or fetch via a lookup) an instance of another class, and make use of it. There's nothing wrong with that, of course, as reusing other classes and objects is the "bread and butter" of object-oriented development. However, rampant use of this approach can sometimes lead to a tightly coupled design, where it's difficult or impossible to swap in a different instance, or even a new implementation. As well as making code difficult to extend, this also makes unit testing a bit of a chore.

The following code is really the heart of HotelPriceCalculator—it's the method that calculates the price to stay at a hotel for the specified number of nights:

public BigDecimal calculatePrice(Hotel hotel, int numNights) {
        Object id = hotel.getId();
        PriceStreamer streamer = (PriceStreamer)
SimpleRegistry.lookup.get(PriceStreamer.class);
        BigDecimal pricePerNight = streamer.quotePrice(id);
        return pricePerNight.multiply(new BigDecimal(numNights));
    }

The code relies on a PriceStreamer having already received at least one price update for the hotel ID in question. Let's create a test for this important method and see what happens:

@Before
public void setUp() throws Exception {
    calc = new HotelPriceCalculator();
    calc.initPriceStreamer();
}

@Test
public void calculatePrice() throws Exception {
    Hotel hotel = new Hotel("123");

    // Give the PriceStreamer a price update:
    PriceStreamer streamer = new PriceStreamer();
    streamer.update("123", new BigDecimal(225.0));

    // Calculate the total price:
    BigDecimal price = calc.calculatePrice(hotel, 3);

    assertEquals(new BigDecimal(675.0), price);
}

By rights, this should be a perfect steal: an instant green bar, a ticket for home at 5 p.m. on the nose. But instead, running the test in Eclipse gives us the Red Bar of Grisly Stay-Late Misery (see Figure 9-6).

Figure 9-6. Red bar of "D'oh!"

As the screenshot shows, the red bar is due to a NullPointerException. Check the test method again... in that code we're creating our own PriceStreamer instance, but it isn't being passed in to HotelPriceCalculator. So we end up with two PriceStreamers (one in the test, one in HotelPriceCalculator), and the simulated price update that we send goes to the wrong one.

As you can see from the calculatePrice() method, the code looks up its copy of PriceStreamer from the SimpleRegistry that we created earlier in the chapter. A quick solution would be to make the test use SimpleRegistry itself... problem solved (kind of):

// Give the PriceStreamer a price update:
PriceStreamer streamer = (PriceStreamer)
        SimpleRegistry.lookup.get(PriceStreamer.class);
streamer.update("123", new BigDecimal(225.0));

Running this gives us a nice green bar, which, in a "Shangri-La" type of world, would indicate that all's well. But, for the same reasons that we discussed earlier, this isn't ideal. It's a fragile test, relying on a static (i.e., system-wide) field inside SimpleRegistry, which we hope (but can't guarantee) won't be modified elsewhere. The test passes today, but tomorrow it might fail because it's vulnerable to external changes.

Another issue—one that leads us neatly to the solution—is that the test we're writing is meant to be about HotelPriceCalculator, not about PriceStreamer (that gets its own set of tests). If the implementation of PriceStreamer changes, the HotelPriceCalculator tests might break in some way: so we're back to the issue of fragile tests. (Note that this isn't so much the case with behavioral tests and acceptance tests, as these types of tests are broader-grained and more about integration of classes and components.)

As the Mapplet UI code is all written using Flex, we'll switch to Adobe-land to illustrate the problem of business logic in the UI code. Figure 9-7 shows a screenshot of what we're trying to achieve: enter some search criteria such as hotel name and number of nights, and click Search. The Flex app will make a call to the server-side code, which will do its overly complex, under-designed, over-engineered, Temple of Doom thing, and finally return some prices.

Figure 9-7. This front-end just wants to be loved... but there is ugliness behind the scenes.

Here's an excerpt from the Flex MXML mark-up:

<s:TextInput id="txtHotelName" />
<s:TextInput id="txtNights" />

<s:TextArea id="txtResults" text="{results}" editable="false" />
<s:Button id="btnSearch" label="Search" click="btnSearch_clickHandler(event)"/>

<fx:Script>
    <![CDATA[
           import mx.controls.Alert;

           [Bindable]
           private var results:String = "";

           protected function btnSearch_clickHandler(event:MouseEvent):void

{
               // Perform validation before submitting the search:

                if (txtHotelName.text.length==0)
                {
                    Alert.show("Please enter a hotel name", "Hotel Name Required");
                }
                else if (txtNights.text.length==0)
                {
                    Alert.show("Please enter the number of nights", "Number of Nights
Required");
                }

                // Create a hotelSearch object and populate it from the input fields:

                var hotelSearch: Object = new Object();
                hotelSearch.hotelName = txtHotelName.text;
                hotelSearch.numNights = txtNights.text;

                // (code omitted) - call the Java PriceCalculator service,
                // and populate 'results' var with the result.

            }

        ]]>
    </fx:Script>

When the user clicks the "Search" button, the click handler method is called. This performs some validation on the input fields, then creates a hotelSearch object, and sends this along to the remote Java service. That all seems fine so far; but what if we want to create a unit test for the validation? Or, what if the customer wants something else to be done to the search fields before they're submitted—e.g., allow "number of nights" to be blank, in which case substitute the value 1 (i.e, default to a one-night search)? The additional code could go into the MXML mark-up shown here. However, the code is becoming less and less object-oriented. All that behavior, and no object to put it in.^[52]

The validation really belongs in a HotelSearch class. We'll show how to fix this up in Chapter 10.

Private methods and members are the bane of a unit tester's life, as the test doesn't have access to them. This isn't as bad of a problem with DDT as it is with TDD, as DDT takes a more "gray box" approach, and doesn't assume a need to know the internals of every line and field within the class under test. However, it still can be an issue:

public class Hotel {
    private boolean hotelDetailsFound;
    . . .

}

Let's say you've written a hotel search test, and you want to assert that the object under test correctly flags that the hotel search was successful:

@Test
public void hotelFoundFlagIsSet() throws Exception {
    Hotel hotel = new Hotel();
    hotel.search("New York", "541 Madison Avenue");
    assertTrue("Hotel was found", hotel.hotelDetailsFound);
}

The code in bold text won't compile, because hotelDetailsFound is private, so the test can't gain access to it. There's more than one possible solution to this issue, with varying degrees of attractiveness. Like a page from www.AmIHotOrNot.com, vote on your favorite solution in Chapter 10, in the section "Use Black Box Testing So You Don't Have to Break Encapsulation."

With Java, fields that are declared final are immutable (note this doesn't mean that values inside the referenced object are immutable; just the reference to the object). Because they're final, it makes sense that they will be initialized as part of the declaration:

final String url = "http://articles.softwarereality.com";
final PriceStreamer priceStreamer = new PriceStreamer();

The first line is an example of a simple value that you want to remain constant, whereas the second line is an example of a complex object, or service object, that performs some task or other for the class under test.

"Simple value" fields that are declared final can generally be considered "a good thing" for the purposes of software quality. You know exactly what you're getting; you don't have to worry about multi-threading issues (e.g., one thread reading the field while another thread is modifying its value; or the race condition where two threads are updating a field, hence one value is lost). However, in the case of service objects, there are occasions when a final member can really put a dampener on a unit test's day.

The just-in-case malady is the bane of developers everywhere, not just those wielding a JUnit test class. It's an insidious antipattern because it's born entirely of good intentions. "I'll make this class reusable (read: "I'll make this class more complex than it needs to be") just in case someone will need to use it later." The result, especially when this "good deed" happens at the coding stage and not at the design or analysis stage, is that the code can't be traced back to a particular business requirement. So when you're maintaining some mystery code—whether to apply unit tests to it, or extend it with new functionality, or refactor a crusty design—it can be difficult to determine whether the code even needs to be there. If the code is simply deleted, will something else in the system mysteriously stop working? It's impossible to say, as it isn't covered by a test or linked back to a requirement.