Our tests run, the code structure is good, yet we are not entirely happy. What's up with that?

Recap

Good morning. As we sit down to pair this morning, we have a failing test:

   @Test
    public void coverSheetHasClientName() {
        ReportPage coverSheet = report.pageNamed("CoverSheet");
        ReportParagraph namePara = coverSheet.paragraphNamed("ClientName");
        assertEquals("Dan Orlich", namePara.text());
    }

And we have enough shell code to make that test fail. We could make it green by just having the default answer be “Dan Orlich”, or by asking for “” in the test. On a given day, we might do that, using “Fake It Till You Make It”. Today we are inclined to go forward. with actual code and make this test run.

However, before we start, Chet asks whether we have bitten off more than we can chew. We are going through a few different objects: Reports, Pages, Paragraphs. We could proceed, for example, by writing a finer-grain test against, say ReportPage, calling for the pageNamed lookup method. For example:

   @Test
    public void reportHasCoverSheet() {
        assertEquals("CoverSheet", report.pageNamed("CoverSheet").name());
    }

This kind of test would take us down the path of implementing the “indexing” facility in PatternReport. Or we could write a test that, given a Paragraph, tested whether we could return the text. Not very interesting. Or … we could just go ahead and try to make our existing test run, without the additional test shown above. We talk about whether to save this test or remove it, and we’re inclined to remove it.

We conclude that there is some risk that we are about to take too big a bite. If so, we’ll find out soon, and do something about it. As the redneck said: “Watch this.”

Quick Design Session

We talk a bit further about how to proceed. There’s a fair amount to do. We have to create a folder named, say, “orlich”, put some kind of data file in it that contains Orlich’s name, build a PatternReport, put a cover sheet Page in it, put a paragraph in it named “ClientName”, with the right text. That’s a lot. Could be risky. But our plan is to go ahead, on the grounds that if anything gets tough we can recur into a simpler test then. We need to expand the existing test to tell it to look for the “orlich” data, and then construct all the objects. We’ll program all that by intention. We enhance the test:

   @Test
    public void coverSheetHasClientName() {
        report.process("orlich");
        ReportPage coverSheet = report.pageNamed("CoverSheet");
        ReportParagraph namePara = coverSheet.paragraphNamed("ClientName");
        assertEquals("Dan Orlich", namePara.text());
    }

Now we “just” have to write the process method. We have decided to fake that method, at least partially. Hold on …

… and on, and on …

Forty-five minutes later we are somewhere almost decent. Recall that our standard time between green bars is ten minutes. If we had reminded ourselves that we wanted a green bar in ten minutes, we would not have undertaken to do it as we have. And we aren’t to a green bar yet, though we are nearly there, and with a structure that isn’t bad for now. Here’s the code. We’ll discuss it on the other side:

package com.hendricksonxp.patterning.model;

import java.util.HashMap;

public class PatternReport {
    HashMap<String, ReportPage> pageMap = new HashMap<String, ReportPage>();

    public HashMap pages() {
        return pageMap;
    }

    public ReportPage pageNamed(String pageName) {
        return pageMap.get(pageName);
    }

    public void process(String folderName) {
        createCoverSheet(folderName);
        // other methods TBD
    }

    private void createCoverSheet(String folderName) {
        ReportPage coverSheet = ReportPage.makeCoverSheet(folderName);
        pageMap.put("CoverSheet", coverSheet);
    }   
}
package com.hendricksonxp.patterning.model;

import java.util.HashMap;

public class ReportPage {
    HashMap<String, ReportParagraph> paragraphMap = new HashMap<String, ReportParagraph>();

    public ReportParagraph paragraphNamed(String paraName) {
        return paragraphMap.get(paraName);
    }

    public static ReportPage makeCoverSheet(String folderName) {
        ReportPage coverSheet = new ReportPage();
        coverSheet.fillInCoverInfo(folderName);
        return coverSheet;
    }

    private void fillInCoverInfo(String folderName) {
        ReportParagraph namePara = new ReportParagraph();
        namePara.setClientName(folderName);
        paragraphMap.put("ClientName", namePara);
    }
}
package com.hendricksonxp.patterning.model;

public class ReportParagraph {
    String text = "uninitialized";

    public String text() {
        return text;
    }

    public void setClientName(String folderName) {
        text = folderName;
    }
}

What we see here isn’t much code given that it took 45 minutes. What is behind the code isn’t that impressive either, but there is some pretty clean design and structure. We have new classes PatternReport, ReportPage, and ReportParagraph, and the outer classes include an indexing structure that is actually used to store and retrieve the page and paragraph we need in order to make our test run.

I’m not sure why these few lines took us 45 minutes to write. No doubt in that period we became distracted by things at the coffee shop and so on, but mostly we were heads down pairing, figuring out what to do and typing it in. When we got this far, to a test that would run to completion and get a red bar, it was 45 minutes later. We were in a small rat hole and didn’t really notice.

When we got to the point of needing the HashMap, that was the first moment when we noticed we were in trouble. If we had set a timer to ping at ten minutes, we would have reset it once by then, which would have been a stronger clue. But we weren’t really in deep trouble: it just felt like we were running our brains too hot. We were operating at that point where we all feel “OK, I can get away with this, but I wouldn’t trust those other guys to push this far, they’re just not up to it.” Our best guess is that we shouldn’t trust ourselves either. However, we did go right straight through with no debugging, and the code structure looks good to us. We could pat ourselves on the back for getting through it.

But it didn’t feel good. When we get to a break, we always try to talk about how things went, because that’s how we keep ourselves sharp and how we adjust our path for next time. This bit felt kind of awkward. We’ll do better tomorrow.

And Tomorrow ...

We have our test getting a legitimate red bar, because we are not filling in the actual client information that the test calls for. The information that the client’s name is “Dan Orlich” is not recorded on the computer anywhere, and we have to record it, and access it.

In our code above, we had decided to pass the name “orlich” around, representing somehow all the information we had on the client orlich. A key to the database if you will. Passing the key around seems wrong. Today, we are planning to replace the folderName that we’re passing around with a ClientInfo object, which will encapsulate the information that is represented in the client folder, such as the “orlich” folder that we are imagining.

Because we have clearly taken too big a bite (who knew??), we’re going to TDD the ClientInfo object directly, then plug it in. That will make the current test go green. According to us.

Begin with a test. We’ll write a test that creates a ClientInfo, using the key “orlich”, and asserting that it knows clientName to be “Dan Orlich”:

package com.hendricksonxp.patterning.test;
import static org.junit.Assert.*;

import org.junit.Test;

import com.hendricksonxp.patterning.model.ClientInfo;

public class ClientInfoTest {

    @Test
    public void clientNameIsDanOrlich() {
        ClientInfo info = new ClientInfo("orlich");
        assertEquals("Dan Orlich", info.clientName());
    }
}

To get this to red, we just let Eclipse create the class, and return the “wrong” answer:

package com.hendricksonxp.patterning.model;

public class ClientInfo {

    public ClientInfo(String folderName) {
    }

    public String clientName() {
        return "wrong";
    }

}

Time Passes ...

We found it necessary to break right above, eat lunch, and return to our respective homes. We’re back now. What shall we do? Fortunately we have the broken tests, and these notes.

Our basic vision – admittedly and intentionally partial – is that there will be a folder somewhere in the system, containing subfolders for each pattern client, containing files with the information we need. So we’ll begin by creating that structure on Chet’s hard drive, including the “orlich” folder, and a file, presently named “customer.txt”, containing, on its first line, the string “Dan Orlich”. <pause/> That’s done. Now we just have to make our CustomerInfo object work. Here goes:

public class ClientInfo {
    final String folder = "Data\\ShotgunPatternData\\";
    String clientName;

    public ClientInfo(String folderName) {
        String fileName = folder + folderName +  "\\customer.txt";
        try {
            BufferedReader in = new BufferedReader(new FileReader(fileName));
            clientName = in.readLine();
            in.close();
        } catch (IOException e) {
        }
    }

    public String clientName() {
        return clientName;
    }
}

No big deal: we just open the file, read the first line, and save it as clientName. The test is green. Now, we claim, if we plug the clientInfo into our existing code, the other test should work as well.

   @Test
    public void coverSheetHasClientName() {
        report.process("orlich");
        ReportPage coverSheet = report.pageNamed("CoverSheet");
        ReportParagraph namePara = coverSheet.paragraphNamed("ClientName");
        assertEquals("Dan Orlich", namePara.text());
    }
public class PatternReport {
    HashMap<String, ReportPage> pageMap = new HashMap<String, ReportPage>();

    public HashMap pages() {
        return pageMap;
    }

    public ReportPage pageNamed(String pageName) {
        return pageMap.get(pageName);
    }

    public void process(String folderName) {
        createCoverSheet(folderName);
        // other methods TBD
    }

    private void createCoverSheet(String folderName) {
        ReportPage coverSheet = ReportPage.makeCoverSheet(folderName);
        pageMap.put("CoverSheet", coverSheet);
    }   
}

This looks a bit extensive but in fact it was straightforward and easy. We created a ClientInfo, then told Eclipse to change the signature of the createCoverSheet method:

   public void process(String folderName) {
        ClientInfo info = new ClientInfo(folderName);
        createCoverSheet(info);
        // other methods TBD
    }

    private void createCoverSheet(ClientInfo info) {
        ReportPage coverSheet = ReportPage.makeCoverSheet(info);
        pageMap.put("CoverSheet", coverSheet);
    }   
}

Eclipse’s little red x’s and hints dragged us through all the other changes. We just kept telling it to change signatures, down to the end:

public class ReportPage {
    HashMap<String, ReportParagraph> paragraphMap = new HashMap<String, ReportParagraph>();

    public ReportParagraph paragraphNamed(String paraName) {
        return paragraphMap.get(paraName);
    }

    public static ReportPage makeCoverSheet(ClientInfo info) {
        ReportPage coverSheet = new ReportPage();
        coverSheet.fillInCoverInfo(info);
        return coverSheet;
    }

    private void fillInCoverInfo(ClientInfo info) {
        ReportParagraph namePara = new ReportParagraph();
        namePara.setClientName(info);
        paragraphMap.put("ClientName", namePara);
    }
}

… and finally …

public class ReportParagraph {
    String text = "uninitialized";

    public String text() {
        return text;
    }

    public void setClientName(ClientInfo info) {
        text = info.clientName();
    }
}

Not many changes, and all except typing in “.clientName()” was automatic.

Our tests are all green. We noticed that I had declared clientName without a modifier, which defaulted it to public (brilliant, Sun), so we fixed that.

Where Are We?

We have the first element of variable data for our cover sheet, and we have a rudimentary but decent structure for client data. All the information is encapsulated in one class, ClientInfo. Any changes to the locations of folders (or even changing the folders to database records), we’ll have one place to change it.

We accomplished that, not by any kind of grand design, but rather by focus on simple code and separation of concerns.

There is some duplication. The Report and Page each have a HashMap of their sub-elements. And the code is similar between the two classes. Each has a top-level method that will wind up doing a series of actions, each of which will do a “put” of a keyed value into the object.

We observe this duplication but elect not to resolve it yet. We see some possible ways to resolve it. There might be a subclass-superclass relationship growing here, some kind of abstraction for an indexed something, along the lines of the Composite pattern. Or there may be some kind of common object that all these guys use, perhaps just representing the indexing. There might even be some data structure, using a Command pattern, perhaps, to reflect the whole document structure in a single place. That would be nice.

We think it’s too soon to do anything about this. Some people say that as soon as duplication emerges, it should be resolved. Others suggest waiting until you have triplication, if that’s a word. Whatever we call it, we’re going to wait and see what happens next.

Retrospective

Technical Results

Recapping where we wound up, we have created model classes representing the structure of a report as we now understand it, Paragraphs within Pages within Report. We expect that will change: in particular we expect that there will be pictures, and probably various kinds of paragraph properties, and so on. We don’t have stories for those, so we didn’t provide for them.

We have a parameter object to pass around all the reporting entities, which is slated to contain all the client-dependent information. At this point we are imagining that it represents the file structure and files that the system will create, but we’re open to that changing. It’s encapsulated well enough for now, so we’re in good shape.

Process Assessment

Things wound up OK and we didn’t have a lot of rework or debugging. On the face of it, we could argue that everything went well. But we don’t feel that it did. We’re pretty sensitive to how our work feels, when it is flowing and when it isn’t, and this series of sessions felt a bit draggy. It’s not clear why.

We had plenty of little design sessions, and our design ideas held together remarkably well. We had no problem bugs, no debugging at all. There were only two signs of trouble. First, we went about 45 minutes without getting a test green on one of the days. That’s a long time and we felt pretty nervous part way through. Second, it just didn’t feel good, for the whole period we worked on this. Very odd.

We did talk about what we might have done differently. We can’t go back and do things differently … unless we were to delete the code and do it another way, and I don’t think we’ll do that this time. But we can think about alternatives from this time, and perhaps work a bit differently next time.

We could have gone bottom up at some point, building the ClientInfo. At first, though, we didn’t know we needed it. We just had the token “orlich”, which was standing in for some vague idea of the customer’s data. Our thoughts at that time were focused on the report - page - paragraph, and we could see that we could defer the information question.

We could have written a smaller test for the keyed lookup – we definitely paused when we got to it. Better yet, we might have gone deeper in faking it. There was no real reason to do the lookup on the orlich test, nor the I/O. That might have laid in a bit more of the object structure, and then a second or third test could have put in the hashing and so on.

Everything that I’m seeing says that the results are quite good enough for now. Yet it felt a bit as if we were lumbering along, not moving with that smooth style and grace we strive for. Looking back, I see no big mistakes.

Bottom line, for me, right now: it was too big a bite for comfort, though not too big to chew. We’ll try to ask ourselves for a while, “can we make this test run in ten minutes”, and be guided by the answer. Stay tuned.