[MO-java-dev] CVS: no-stone-unturned/doc/xdocs servlets_2.xml,NONE,1.1 servlets_1.xml,NONE,1.1 doc-book.xml,1.6,1.7

[Steve F. <sm...@us...>]

Update of /cvsroot/mockobjects/no-stone-unturned/doc/xdocs
In directory usw-pr-cvs1:/tmp/cvs-serv10470/doc/xdocs

Modified Files:
	doc-book.xml 
Added Files:
	servlets_2.xml servlets_1.xml 
Log Message:
ported servets 1 & 2 chapters

--- NEW FILE: servlets_2.xml ---
<chapter>
  <title>Test-driven servlets (ii)</title>

  <section>
    <title>Introduction</title>

    <para>
      In the previous chapter, I showed how to develop a simple servlet
      from tests. Our customers were so happy with the initial release that
      they approved another development cycle. This time, they want to
      improve the presentation of the results. After some discussion, we
      decide that the most important tasks to implement are:
    </para>

    <procedure>
      <step><para>
        display the calculation details in the ouptut, as well as the result
      </para></step>
      <step><para>
        if the request includes a <literal>format=csv</literal> parameter, then
        write the output as comma separated values and change the content
        type. Otherwise, write the output as before.
      </para></step>
    </procedure>

    <para>
      The new requirements suggest that we may have to factor out the
      part of the application that displays the results, but let's not
      commit ourselves just yet. Instead, we can implement the first task as
      simply as possible, and then see where the code takes us.
    </para>
  </section> <!-- Introduction -->

  <section>
    <title>Display the calculation details</title>

    <section>
      <title>The first test</title>

      <para>
        Adding the calculation details to the page should be
        straightforward. We can adapt one of our tests to something like:
      </para>

      <programlisting>
public class CalculatorServletTest
  <emphasis>[...]</emphasis>
  public void testSubtractTwoIntegers() {
    MockHttpServletRequest mockRequest = createMockRequest("3", "5", "/add");

    mockResponse.setExpectedContentType("text/plain");
    calculatorServlet.doGet(mockRequest, mockResponse);

    <emphasis>checkPageContents("3 /add 5", "Response page calculation");</emphasis>
    checkPageContents("Result is 8", "Response page result");
  }</programlisting>

      <para>
        Unfortunately, our current implementation of <function>checkPageContents()</function> matches
        the generated page exactly, so this test won't work because we have to verify two lines of
        output. We could write something that would extract and compare each line of the page, but I
        think we can do something simpler. If we change <function>checkPageContents()</function> to check
        whether the expected string is <emphasis>included</emphasis> in the output page, then this test becomes
        easy. Of course, this means that we would be giving up some precision because we wouldn't know where
        in the page each fragment occurs; for example, we cannot assert that the calculation details come before the
        result in the page. I'm happy with this because, for now, I just want to know that the servlet calculates
        the right values. We'll change <function>checkPageContents()</function> as follows and, because its
        behaviour has changed, rename it as well.
      </para>

      <programlisting>
private void assertPageContains(String expected, String message) {
  String actualPage = mockResponse.getWrittenPage();
  if (actualPage.indexOf(expected) == -1) {
    fail(message + " expected &lt;" + actualPage + "&gt; to include &lt;" + expected + "&gt;");
  }
}</programlisting>

      <para>Now we have a breaking test, so we know what to do.</para>

      <programlisting>
Testcase: testAddTwoIntegers took 0 sec
  FAILED
Response page calculation expected &lt;Result is 8
&gt; to include &lt;3 /add 5&gt;
  at tdd.CalculatorServletTest.assertPageContains(CalculatorServletTest.java:57)
  at tdd.CalculatorServletTest.testAddTwoIntegers(CalculatorServletTest.java:23)</programlisting>

      &redbar;

      <para>The fix is simple, we add another print line to <function>doGet()</function></para>

      <programlisting>
response.setContentType("text/plain");
PrintWriter wr = response.getWriter();
try {
  int value1 = Integer.parseInt(request.getParameter("value1"));
  int value2 = Integer.parseInt(request.getParameter("value2"));
  String operation = request.getPathInfo();

  <emphasis>wr.println(value1 + " " + operation + " " + value2);</emphasis>
  wr.println("Result is " + binaryOp(operation, value1, value2));
} catch (IllegalArgumentException ex) {
  wr.println("Error: " + ex.getMessage());
}</programlisting>

      <para>
        whicb passes the test. This looks easy, so let's get on and fix the
        other tests. <function>testSubtractTwoIntegers()</function> and
        <function>testBadOperation()</function> work with the same implementation.
        Next we try <function>testValueNotAnInteger()</function>:
      </para>

      <programlisting>
Testcase: testValueNotAnInteger took 0 sec
  FAILED
Response page calculation expected &lt;Error: not an int
&gt; to include &lt;3 /add not an int&gt;
  at tdd.CalculatorServletTest.assertPageContains(CalculatorServletTest.java:60)
  at tdd.CalculatorServletTest.testValueNotAnInteger(CalculatorServletTest.java:53)</programlisting>

      &redbar;

      <para>
        Oops! When parsing an integer fails, the exception bypasses the
        line that prints the calculation details. I'm glad we had a test for
        what seemed like an obvious fix. We should extract the parameters as
        strings, to print the request, and <emphasis>then</emphasis> convert them to
        integers.
      </para>

      <programlisting>
response.setContentType("text/plain");
PrintWriter wr = response.getWriter();
try {
  <emphasis>String value1Param = request.getParameter("value1");
  String value2Param = request.getParameter("value2");</emphasis>
  String operation = request.getPathInfo();

  wr.println(value1Param + " " + operation + " " + value2Param);

  <emphasis>int value1 = Integer.parseInt(value1Param);
  int value2 = Integer.parseInt(value2Param);</emphasis>
  wr.println("Result is " + binaryOp(operation, value1, value2));
} catch (IllegalArgumentException ex) {
  wr.println("Error: " + ex.getMessage());
}</programlisting>

      &greenbar;

      <para>
        Now it passes. We could just ship it, but the deal that allows us
        to do simple things to get through the test also requires us to clean
        up the mess before we go. Time to pay up.
      </para>
    </section>

    <section>
      <title>Interlude: Refactoring</title>

      <para>
        I'll show these two refactorings together but, trust me, I really
        did implement them one at a time and, of course, I ran the tests after
        every change. First, we notice that the code for getting values from
        the request won't throw an <classname>IllegalArgumentException</classname>, so
        we move this code out of the <token>try</token> block. If we don't, one
        day some other programmer (probably us when we've forgotten) will have
        to spend time trying to figure out whether we knew about some obscure
        failure condition or we just made a mistake.
      </para>

      <para>
        Next, the code for converting to integers is a bit repetitive. In
        nicer languages, we would just extend the <classname>String</classname> class,
        but here let's keep it simple and write a little helper method
        <function>asInt()</function>. With some variable renaming to improve
        readibility, we now have:
      </para>

      <programlisting>
protected void doGet<emphasis>[...]</emphasis>
  <emphasis>String value1 = request.getParameter("value1");
  String value2 = request.getParameter("value2");
  String operation = request.getPathInfo();</emphasis>

  response.setContentType("text/plain");
  PrintWriter wr = response.getWriter();

  <emphasis>wr.println(value1 + " " + operation + " " + value2);</emphasis>
  try {
    wr.println("Result is "
            + binaryOp(operation, <emphasis>asInt(value1)</emphasis>, <emphasis>asInt(value2)</emphasis>));
  } catch (IllegalArgumentException ex) {
    wr.println("Error: " + ex.getMessage());
  }
}

<emphasis>private int asInt(String valueString) {
  return Integer.parseInt(valueString);
}</emphasis></programlisting>

    </section>

    <section>
      <title>What have we learned?</title>

      <para>
        Even simple changes (or, perhaps, <emphasis>especially</emphasis> simple
        changes) can create flaws in our logic. If we keep them up to date,
        the unit tests will save us from our mistakes. We also have to think
        carefully about what we're testing. The tests for the generated page
        would have become messier once we'd added a second line. We made the
        tradeoff that checking that a string is <emphasis>included</emphasis> in the page,
        rather than an exact match, gives us enough confidence that the code
        still works and makes the tests much easier to write. Finally, we
        musn't forget that we have a debt of honour when developing test-first
        to keep the code clean and tidy before we move on. The final version
        expresses clearly what we expect the output to be: the details of the
        request, and either a result or an error. It's harder to see this in
        the unrestructured version.
      </para>
    </section>
  </section> <!-- Display the calculation details -->

  <section>
    <title>Comma separated values</title>

    <section>
      <title>The first test</title>

      <para>
        Now we have to add another format for the output page if the end
        user sets the <varname>format</varname> parameter. We consult our customers
        and they decide that they want the calculation description and result
        on the same line. We add a new test that sets the parameter and checks
        the new output.
      </para>

      <programlisting>
public void testAddTwoIntegersCsv() throws ServletException, IOException {
  MockHttpServletRequest mockRequest = createMockRequest("3", "5", "/add", <emphasis>"csv"</emphasis>);

  mockResponse.setExpectedContentType("text/<emphasis>csv</emphasis>");
  calculatorServlet.doGet(mockRequest, mockResponse);

  assertPageContains(<emphasis>"3,/add,5,8"</emphasis>, "Response page csv addition");
}</programlisting>

      <para>
        To make this work, we've extended <function>createMockRequest()</function>
        to add a format parameter and overloaded the original version to call
        the new one with a null value, so we don't have to change out existing
        tests. The implementation of the <classname>MockHttpServletRequest</classname>
        is becoming a bit messy but we'll live with that for now.
      </para>

      <programlisting>
private MockHttpServletRequest
  <emphasis>createMockRequest</emphasis>(String value1, String value2, String pathInfo)
{
  return createMockRequest(value1, value2, pathInfo, <emphasis>null</emphasis>);
}
private MockHttpServletRequest
  createMockRequest(final String value1, final String value2,
                    final String pathInfo, <emphasis>final String format</emphasis>)
{
  return new MockHttpServletRequest() {
    public String getParameter(String key) {
      <emphasis>if ("format".equals(key)) return format;</emphasis>
      <emphasis>[...]</emphasis>
    }
  }
}</programlisting>

      <para>Of course the test fails. It tells us to fix the content type.</para>

      <programlisting>
Testcase: testAddTwoIntegersCsv took 0 sec
  FAILED
Content type expected:&lt;text/csv&gt; but was:&lt;text/plain&gt;
  at tdd.MockHttpServletResponse.setContentType(MockHttpServletResponse.java:21)
  at tdd.CalculatorServlet.doGet(CalculatorServlet.java:15)</programlisting>

      &redbar;

      <para>We look for the request parameter and change the content type if we need to.</para>

      <programlisting>
protected void doGet(
  <emphasis>[...]</emphasis>
  <emphasis>if ("csv".equalsIgnoreCase(request.getParameter("format"))) {
    response.setContentType("text/csv");
  } else {</emphasis>
    response.setContentType("text/plain");
  <emphasis>}</emphasis>
  <emphasis>[...]</emphasis></programlisting>

      <para>We run the tests again and now the output string is wrong.</para>

      <programlisting>
Testcase: testAddTwoIntegersCsv took 0 sec
  FAILED
Response page calculation expected &lt;3 /add 5
Result is 8
&gt; to include &lt;3,/add,5,8&gt;
  at tdd.CalculatorServletTest.assertPageContains(CalculatorServletTest.java:69)
  at tdd.CalculatorServletTest.testAddTwoIntegersCsv(CalculatorServletTest.java:63)</programlisting>

      &redbar;

      <para>
        I have a small dilemma here. I need to branch both the content type
        and the content based on the format parameter. Should I hang on the
        decision and write multiple <token>if</token> statements, or have just
        one <token>if</token> statement but duplicate the code that both
        branches use? Surely this is an important design decision but I can't
        think of an obvious advantage for either of the choices, so I reach
        into my toolbox for my Executive Decision Support System. It comes up
        Heads, so we're going with the first option.
      </para>

      <programlisting>
protected void doGet(
  <emphasis>[...]</emphasis>
  <emphasis>boolean isCsvFormat = "csv".equalsIgnoreCase(request.getParameter("format"));</emphasis>
  if (<emphasis>isCsvFormat</emphasis>) {
    response.setContentType("text/csv");
  } else {
    response.setContentType("text/plain");
  }
  PrintWriter wr = response.getWriter();

  <emphasis>if (isCsvFormat) {
    wr.println(value1 + "," + operation + "," + value2 + ","
               + binaryOp(operation, asInt(value1), asInt(value2)));
  } else {</emphasis>
    wr.println(value1 + " " + operation + " " + value2);
    try {
      wr.println("Result is "
                 + binaryOp(operation, asInt(value1), asInt(value2)));
    } catch (IllegalArgumentException ex) {
      wr.println("Error: " + ex.getMessage());
    }
  <emphasis>}</emphasis>
}</programlisting>

      &greenbar;

    </section>

    <section>
      <title>Handling errors</title>

      <para>
        We had a nasty surprise last time when tested for a non-integer
        value, so let's do that one next. Incidentally, our customers have
        asked us not to put results and errors in the same column, so there's
        an extra comma for the missing result value.
      </para>

      <programlisting>
public void <emphasis>testValueNotAnIntegerCsv</emphasis>() throws ServletException, IOException {
  MockHttpServletRequest mockRequest = createMockRequest("3", "not an int", "/add", "csv");

  mockResponse.setExpectedContentType("text/csv");
  calculatorServlet.doGet(mockRequest, mockResponse);

  assertPageContains(<emphasis>"3,/add,not an int,,not an int"</emphasis>, "Not an integer error, CSV");
}</programlisting>

      <para>
        We need to shuffle our output statements to be included in the
        exception logic. It's a bit messy, but it makes the tests pass.
      </para>

      <programlisting>
protected void doGet(
  <emphasis>[...]</emphasis>
  if (isCsvFormat) {
      <emphasis>wr.print(value1 + "," + operation + "," + value2 + ",");</emphasis>
  } else {
      wr.println(value1 + " " + operation + " " + value2);
  }
  try {
    <emphasis>if (isCsvFormat) {
      wr.println(binaryOp(operation, asInt(value1), asInt(value2)));
    } else {</emphasis>
      wr.println("Result is "
                 + binaryOp(operation, asInt(value1), asInt(value2)));
    <emphasis>}</emphasis>
  } catch (IllegalArgumentException ex) {
    <emphasis>if (isCsvFormat) {
      wr.println("," + ex.getMessage());
    } else {</emphasis>
      wr.println("Error: " + ex.getMessage());
    <emphasis>}</emphasis>
  }
}</programlisting>

      &greenbar;

    <para>
      This implementation also works for CSV version of the <emphasis>subtract</emphasis>
      and <emphasis>unknown operation</emphasis> tests.
    </para>
  </section>

  <section>
    <title>What have we learned</title>

    <para>
      We've added a new area of functionality without ever spending more
      than a couple of minutes without working tests. We've have accumulated
      enough infrastructure that it's becoming easy to add new tests and
      code. It's also pretty clear that there should be some good candidates
      for refactoring in all the repeated <token>if</token> statements. One of
      the skills in test-driven development is learning that sometimes you
      have to wait until the code is really clear about how it should be
      refactored. While working on these examples, I followed several false
      trails that weren't justified by the code because I was too keen to
      implement various clever abstractions. I still have an absolute
      commitment to releasing only clean, expressive code, but now I wait a
      little longer than I used to before starting larger
      refactorings. Finally, I really did mean that bit about flipping a
      coin; sometimes you just need to take a decision so you can write code
      and gain more experience. In Test Driven Development this is cheap
      because you're never far from a working system and with frequent
      releases it's easy to back off a weak choice.
    </para>
    </section>
  </section> <!-- Comma separated values -->

  <section>
    <title>Interlude: Refactoring</title>

    <para>
      I'm about to embark on a relatively large refactoring. There'll be
      a lot of code but hang on for the ride, because it'll take us in some
      interesting new directions.
    </para>

    <section>
      <title>The print statements</title>

      <para>
        I hope you find it screamingly obvious that we should do something
        about the multiple <token>if</token> statements. Let's start with the
        print statements, we'll pull each one out into a helper method. It's a
        minor change, but it helps us to clarify our ideas about what things
        should be called.
      </para>

      <programlisting>
<emphasis>[...]</emphasis>
if (isCsvFormat) {
  printBinaryOpCsv(wr, value1, operation, value2);
} else {
  printBinaryOpPlain(wr, value1, operation, value2);
}
try {
  if (isCsvFormat) {
    printResultCsv(wr, binaryOp(operation, asInt(value1), asInt(value2)));
  } else {
    printResultPlain(wr, binaryOp(operation, asInt(value1), asInt(value2)));
  }
} catch (IllegalArgumentException ex) {
  if (isCsvFormat) {
    printErrorCsv(wr, ex.getMessage());
  } else {
    printErrorPlain(wr, ex.getMessage());
  }
}</programlisting>

      <para>
        It looks like there are two sets of very similar methods, but
        they're scattered across the code. Let's work on the structure by
        creating a couple of new classes, <classname>CsvCalculatorWriter</classname> and
        <classname>PlainCalculatorWriter</classname>, and attaching the relevant methods
        to each one.
      </para>

      <programlisting>
public class CalculatorServlet
  <emphasis>[...]</emphasis>
  protected void doGet(
  <emphasis>[...]</emphasis>
    <emphasis>CsvCalculationWriter csvWriter = new CsvCalculationWriter();
    PlainCalculationWriter plainWriter = new PlainCalculationWriter();</emphasis>
    if (isCsvFormat) {
      <emphasis>csvWriter.binaryOp(</emphasis>wr, value1, operation, value2);
    } else {
      <emphasis>plainWriter.binaryOp(</emphasis>wr, value1, operation, value2);
    }
    try {
      if (isCsvFormat) {
        <emphasis>csvWriter.result(</emphasis>wr, binaryOp(operation, asInt(value1), asInt(value2)));
      } else {
        <emphasis>plainWriter.result(</emphasis>wr, binaryOp(operation, asInt(value1), asInt(value2)));
      }
    } catch (IllegalArgumentException ex) {
      if (isCsvFormat) {
        <emphasis>csvWriter.error(</emphasis>wr, ex.getMessage());
      } else {
        <emphasis>plainWriter.error(</emphasis>wr, ex.getMessage());
      }
    }
}

<emphasis>public class CsvCalculationWriter {
  public void binaryOp(PrintWriter wr, String value1, String operation, String value2) {
    wr.print(value1 + "," + operation + "," + value2 + ",");
  }
  public void result(PrintWriter wr, int result) {
    wr.println(result);
  }
  public void error(PrintWriter wr, String message) {
    wr.println("," + message);
  }
}</emphasis>
<emphasis>[...]</emphasis></programlisting>

    <para>
      Clearly the two writer classes are nearly identical, so we can
      extract a common parent <classname>CalculationWriter</classname>
    </para>

    <programlisting>
    <emphasis>[...]</emphasis>
    CalculationWriter csvWriter = new CsvCalculationWriter();
    CalculationWriter plainWriter = new PlainCalculationWriter();
    <emphasis>[...]</emphasis>

public class CsvCalculationWriter <emphasis>extends CalculationWriter</emphasis> { <emphasis>[...]</emphasis>
public class PlainCalculationWriter <emphasis>extends CalculationWriter</emphasis> { <emphasis>[...]</emphasis>

<emphasis>public abstract class CalculationWriter {
  public abstract void binaryOp(PrintWriter wr, String value1, String operation, String value2);
  public abstract void result(PrintWriter wr, int result);
  public abstract void error(PrintWriter wr, String message);
}</emphasis></programlisting>

    <para>
      Now, finally, we can collapse the duplicated <token>if</token> statements and let polymorphism do the work.
    </para>

    <programlisting>
public class CalculatorServlet
  <emphasis>[...]</emphasis>
  protected void doGet(
  <emphasis>[...]</emphasis>
    <emphasis>CalculationWriter calcWriter = createCalculationWriter(isCsvFormat);</emphasis>
    calcWriter.binaryOp(wr, value1, operation, value2);
    try {
      <emphasis>calcWriter</emphasis>.result(wr, binaryOp(operation, asInt(value1), asInt(value2)));
    } catch (IllegalArgumentException ex) {
      <emphasis>calcWriter</emphasis>.error(wr, ex.getMessage());
    }
  }

  <emphasis>private CalculationWriter createCalculationWriter(boolean csvFormat) {
    if (csvFormat) {
      return new CsvCalculationWriter();
    } else {
      return new PlainCalculationWriter();
    }
  }</emphasis>
}</programlisting>
    </section>

    <section>
      <title>Setting up the writer</title>

      <para>
        So now we're done? Not yet. We still have two <token>if</token>
        statements: one to set the content type and one to choose the
        calculation writer. We split them because we need to create the
        <classname>PrintWriter</classname> from the serlvet response between these two
        actions. If we move the management of the content type into our
        calculation writers, then we can let them set the content type.
      </para>

      <programlisting>
public class CalculatorServlet
  <emphasis>[...]</emphasis>
  protected void doGet(
    <emphasis>[...]</emphasis>
    <emphasis>boolean isCsvFormat = "csv".equalsIgnoreCase(request.getParameter("format"));
    CalculationWriter calcWriter = createCalculationWriter(isCsvFormat);
    calcWriter.setContentType(response);</emphasis>
    PrintWriter wr = response.getWriter();

    calcWriter.binaryOp(wr, value1, operation, value2);
    try {
      calcWriter.result(wr, binaryOp(operation, asInt(value1), asInt(value2)));
    } catch (IllegalArgumentException ex) {
      calcWriter.error(wr, ex.getMessage());
    }
  }

public class CsvCalculationWriter {
  <emphasis>public void setContentType(HttpServletResponse response) {
    response.setContentType("text/csv");
  }</emphasis>
  <emphasis>[...]</emphasis>
}</programlisting>

      <para>
        Looking at this version, the <varname>isCsvFormat</varname> variable is
        redundant, it's only used once. We can pass the request object to the
        <function>createCalculationWriter()</function> and make the decision
        there. This has the nice side-effect that the top-level
        <function>doGet()</function> no longer has to know anything about the output format.
      </para>

      <programlisting>
  <emphasis>[...]</emphasis>
  CalculationWriter calcWriter = createCalculationWriter(<emphasis>request</emphasis>);
  <emphasis>[...]</emphasis>

private CalculationWriter createCalculationWriter(<emphasis>HttpServletRequest request</emphasis>) {
  if (<emphasis>"cvs".equalsIgnoreCase(request.getParameter("format"))</emphasis>) {
    return new CsvCalculationWriter();
  } else {
    return new PlainCalculationWriter();
  }
}</programlisting>
    </section>

    <section>
      <title>Moving the print writer</title>

      <para>
        So <emphasis>now</emphasis> we're done? Well, we could leave it there, but I
        think there's more. Let's take another look at the body of the <function>doGet()</function> method.
      </para>

      <programlisting>
CalculationWriter calcWriter = createCalculationWriter(request);
calcWriter.setContentType(response);
PrintWriter wr = response.getWriter();

calcWriter.binaryOp(wr, value1, operation, value2);
try {
  calcWriter.result(wr, binaryOp(operation, asInt(value1), asInt(value2)));
} catch (IllegalArgumentException ex) {
  calcWriter.error(wr, ex.getMessage());
}</programlisting>

      <para>
        Notice that we're passing exactly the same print writer to the
        calculation writer several times, and that the <varname>wr</varname> and
        <varname>calcWriter</varname> have about the same lifetime. The duplication
        suggests to me that the calculation writer should own the print
        writer. Let's make it an instance variable of
        <classname>CalculationWriter</classname>, as it's used by both the csv and plain
        versions, and remove the <varname>wr</varname> parameters.
      </para>

      <programlisting>
public class CalculatorServlet
  <emphasis>[...]</emphasis>
  protected void doGet(
    <emphasis>[...]</emphasis>
    CalculationWriter calcWriter = createCalculationWriter(request);
    calcWriter.setContentType(response);
    <emphasis>calcWriter.setWriter(response.getWriter());</emphasis>

    calcWriter.binaryOp(<emphasis>value1, operation, value2</emphasis>);
    try {
      calcWriter.result(<emphasis>binaryOp(operation, asInt(value1), asInt(value2))</emphasis>);
    } catch (IllegalArgumentException ex) {
      calcWriter.error(<emphasis>ex.getMessage()</emphasis>);
    }
  }
}

public abstract class CalculationWriter {
  <emphasis>protected PrintWriter wr;
  public void setWriter(PrintWriter aWriter) {
      wr = aWriter;
  }</emphasis>
  public abstract void setContentType(HttpServletResponse response);
  public abstract void binaryOp(<emphasis>String value1, String operation, String value2</emphasis>);
  public abstract void result(<emphasis>int result</emphasis>);
  public abstract void error(<emphasis>String message</emphasis>);
}</programlisting>

      <para>
        At this point, my Java code critic complains that the import of
        <classname>java.io.PrintWriter</classname> into <classname>CalculationWriter</classname>
        is unused. That means that the servlet code knows nothing about
        writing text. We seem to be drifting towards cleaner, more orthogonal
        code just by concentrating on removing local duplications.
      </para>
    </section>

    <section>
      <title>Setting up the response</title>

      <para>
        We're almost at the end of this bit, really we are. It's a subtle
        point, but we've got two methods, <function>setContentType()</function> and
        <function>setWriter()</function> on the calculation writer that (more or less)
        talk to the http response. Can we establish the link between the two
        objects just once? Let's push them into an initialisation method on
        the calculation writer.
      </para>

      <programlisting>
public class CalculatorServlet
  <emphasis>[...]</emphasis>
  protected void doGet(
    <emphasis>[...]</emphasis>
    CalculationWriter calcWriter = createCalculationWriter(request);
    <emphasis>calcWriter.initialize(response);</emphasis>
    <emphasis>[...]</emphasis>
  }
}

public abstract class CalculationWriter {
  protected PrintWriter wr;
  <emphasis>public void initialize(HttpServletResponse response) throws IOException {
    setContentType(response);
    wr = response.getWriter();
  }</emphasis>
  <emphasis>protected</emphasis> abstract void setContentType(HttpServletResponse response);
  public abstract void binaryOp(String value1, String operation, String value2);
  public abstract void result(int result);
  public abstract void error(String message);
}</programlisting>

      <para>
        Notice that we've changed <function>setContentType()</function> from public
        to protected, as it's no longer called from outside the calculation
        writer. Incidentally, the new initialisation code has to be called
        before a <classname>CalculationWriter</classname> is usable, so it looks like it
        might belong in a constructor. I like the idea, but the code does have
        a side effect of setting state in the response object. It's an old
        habit, but I prefer not to change third party objects in
        constructors. Let's suspend that change until we can find a good
        reason to go one way or the other.
      </para>
    </section>

    <section>
      <title>What have we learned?</title>

      <para>
        By changing our code in a series of small, local improvements we've
        arrived at a clean implementation of the top-level servlet code and
        identified a new concept, the calculation writer. I'm presenting the
        final results, but I regularly find that such moves lead me in
        directions I hadn't expected. There are a couple of interesting
        lessons from code in this section. First, sometimes you have to create
        more code to get to less. Introducing the <classname>CsvCalculationWriter</classname> and
        <classname>PlainCalculationWriter</classname> classes took more lines of code,
        but also gave us a route towards removing duplication by showing us
        where we could hang a single decision point&mdash;the common
        <classname>CalculationWriter</classname> parent. I'm really pleased that setting
        of the response content type and creating its writer have ended up in
        the same method. We've made it hard to get this wrong, which is a
        typical bug when writing servlets.
      </para>

      <para>
        Second, it takes a while to get used to just how far you can push
        refactoring. We aren't taught this in college, and most development
        shops actively discourage changing working code. The deal with
        Test-Driven Development, however, is that you're allowed to get away
        with simple first implementations because you also promise to clean up
        afterwards. In return, you get early feedback about what the code is
        supposed to do and you're never far away from a working set of unit tests.
      </para>

      <para>
        One useful technique in this example is trying to remove local
        variables, especially when they're declared in the middle of a
        method. The lifetime of a local variable (from where it's created to
        where it's last used) is a clue that you can extract some kind of
        structure, such as a helper method or even a new object. If you can
        think of a name to describe the block of code, there's a concept there that deserves factoring out.
      </para>
    </section>
  </section> <!-- Interlude: Refactoring -->

  <section>
    <title>Summary</title>

    <para>
      This iteration has been mainly about changing the output and, step
      by step, it's led us to discover the <classname>CalculatorWriter</classname>. In
      the process, we've removed any knowledge of what a calculator writer
      actually does from the servlet method, so our code is getting more
      orthogonal without any dramatic effort on our part. Actually, it's
      probably the same amount of effort as if we'd sat down and figured it
      all out beforehand, but spread throughout the session in a series of
      local decisions. The design points in Test-First Development are in
      deciding what to put in a test and what to refactor next. The
      important advantage is that each decision is based on the experience
      of the problem that we've just gained from writing code, and only has
      to deal with the next immediate step.
    </para>

    <para>
      Many developers find it hard to give up "proper" design. It's what
      we're trained to do and it's a sign of seniority (Principal Architects
      are too valuable to write code). Personally, I think the code is too
      important to be left as a training exercise. As Fred Brooks points out when describing Chief Programmer
      Teams in <citetitle pubwork="book">The Mythical Man-Month</citetitle>, you
      would expect the critical part of your operation to be performed by
      the specialist surgeon you're paying extra for, not just under his or
      her direction. I also find the fine-grained incremental approach of
      Test-Driven Development much less stressful, because each decision is
      local and reversible. I rarely have to commit anything in stone.
    </para>

    <para>
      The other interesting issue is quality. Many developers are shocked
      by the degree of refactoring that Test-Driven Development requires, it
      really is a change from what most of us have been used to. Many
      development shops actively discourage changing code that works because
      the risks are too high (at one of my early jobs, we weren't even
      allowed to change the formatting!). Test-Driven Development says,
      first, that you <emphasis>can</emphasis> change the code because your tests will
      protect you from mistakes and, second, that you <emphasis>must</emphasis> change
      your code if you can improve it because that's how you maintain the
      agility you need to avoid designing ahead. If you don't believe me,
      you can try the experience I've repeatedly been through of taking over
      a conventionally written code base. If you extrapolate the additional
      difficulty of making changes to "normal" code from code that is
      well-factored and tested, you will understand how all those tests and
      "unnecessary" refactorings can slash your ongoing development costs.
    </para>
  </section> <!-- Summary -->

</chapter>

--- NEW FILE: servlets_1.xml ---
<chapter>
  <title>Test-driven servlets (i)</title>

  <section>
    <title>Introduction</title>

    <para>
      Many software systems are event-driven: the application
      infrastructure converts an external event, such as a mouse click or a
      web query, to a software request which it passes to the application
      code. The application then responds to that request by, for example,
      serving up a web page or sending a message.  The obvious way to test
      an event-driven application is to set up a copy of the system, send it
      events, and see what comes back.  This the way to do integration
      testing, but is clumsy for unit testing. It's hard to get the
      fine-grained control over the state of the system to make sure you can
      exercise all the paths, and the test-and-code cycle can take too long
      to run&mdash;especially if you have to restart the server when the
      code changes.
[...985 lines suppressed...]

    <para>
      So far, this example has been very simple. We have a couple of
      calculations with some basic presentation. On the other hand, we have
      found a mechanism for testing servlets without having to go through an
      application server and with tests that are (more or less) readable.
    </para>

    <para>In the next chapter, we will see what happens when the requirements become more complex.</para>
  </section> <!-- Summary -->

  <appendix>
    <title>A brief introduction to servlets</title>

    <para>
      If you're not familiar with Java servlets, here's a brief introduction.
    </para>
  </appendix>

</chapter>
Index: doc-book.xml
===================================================================
RCS file: /cvsroot/mockobjects/no-stone-unturned/doc/xdocs/doc-book.xml,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- doc-book.xml	10 Aug 2002 00:45:31 -0000	1.6
+++ doc-book.xml	10 Aug 2002 12:22:59 -0000	1.7
@@ -6,6 +6,9 @@
  <!ENTITY part_introduction SYSTEM "file://@docpath@/introduction.xml">
  <!ENTITY chp_how_mocks_happened SYSTEM "file://@docpath@/how_mocks_happened.xml">
 
+ <!ENTITY chp_servlets_1 SYSTEM "file://@docpath@/servlets_1.xml">
+ <!ENTITY chp_servlets_2 SYSTEM "file://@docpath@/servlets_2.xml">
+
  <!ENTITY chp_jdbc_testfirst SYSTEM "file://@docpath@/jdbc_testfirst.xml">
 
  <!ENTITY part_testing_guis SYSTEM "file://@docpath@/testing_guis_1.xml">
@@ -40,9 +43,8 @@
   <part>
     <title>Larger examples</title>
 
-    <chapter>
-      <title>Servlets</title>
-    </chapter>
+    &chp_servlets_1;
+    &chp_servlets_2;
 
     &chp_jdbc_testfirst;
   </part>
@@ -51,24 +53,19 @@
 
   <part>
     <title>Living with Unit Tests</title>
-
     <chapter><title>Test organisation</title></chapter>
-
     <chapter><title>Test smells</title></chapter>
-
     <chapter><title>Retrofitting unit tests</title></chapter>
   </part>
 
   <part><title>Some other languages</title>
 
-    <chapter>
-      <title>C++</title>
+    <chapter><title>C++</title>
       <para>with Workshare?</para>
     </chapter>
 
-    <chapter><title>bash</title></chapter>
-
     <chapter><title>Dynamic and metaprogramming</title></chapter>
+    <chapter><title>bash</title></chapter>
   </part>
 
   <part>