With a little time on our hands, we make our sketched story run and smooth out the code. Generality is showing up, almost by magic. Except it isn't magic -- you can do it too!

Our Mission

… and we have decided to accept it … is to make this sketched method actually work:

   public void setText(String pre, String variableKey, String post, ClientInfo info) {
        String variable = info.lookup(variableKey);
        setParagraphText(pre, variable, post);
    }

Without this method, we’re at a green bar. So we decide to refactor a bit. We change this method:

   public void setClientName(ClientInfo info) {        
        setParagraphText("", info.clientName(), "");
    }

… to this …

   public void setClientName(ClientInfo info) {        
        setText("", "clientName", "", info);
    }

That wouldn’t compile, so we implemented lookup on ClientInfo to return “Dan Orlich”. The test went green. Then we changed the setGunName:

   public void setGunName(ClientInfo info) {
        setText("This report refers to your gun, the ", "gunName", ".", info);
    }

That provides a red bar because the gun name isn’t “Dan Orlich”. We enhance ClientInfo as follows:

public class ClientInfo {
    final String folder = "Data\\ShotgunPatternData\\";
    private HashMap<String, String> oracleDatabase = new HashMap<String, String>();

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

    public String lookup(String variableKey) {
        return oracleDatabase.get(variableKey); 
    }
}

In doing this we removed the clientName and gunName member variables and have made the whole thing key-driven. Now what we don’t like is here:

public class ReportParagraph {
    String text = "uninitialized";

    public String text() {
        return text;
    }

    public void setClientName(ClientInfo info) {        
        setText("", "clientName", "", info);
    }

    public void setGunName(ClientInfo info) {
        setText("This report refers to your gun, the ", "gunName", ".", info);
    }

    public void setText(String pre, String variableKey, String post, ClientInfo info) {
        String variable = info.lookup(variableKey);
        setParagraphText(pre, variable, post);
    }

    private void setParagraphText(String pre, String variable, String post) {
        text = pre + variable + post;
    }   
}

The setClientName and setGunName methods are using the customer-defined paragraph contents, and the key values that customer and programmers have agreed upon in looking up client-dependent values. Where we are heading is to have the paragraph contents (and names) separate from the program, in a file that Chet can edit. We begin by removing the clientName and gunName methods as redundant, unnecessary, and not needed any more.

When we remove the clientName() and gunName() methods from clientInfo, our early unit test won’t compile. We change it:

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

And all is good again. Look again at this:

public class ReportParagraph {
    String text = "uninitialized";

    public String text() {
        return text;
    }

    public void setClientName(ClientInfo info) {        
        setText("", "clientName", "", info);
    }

    public void setGunName(ClientInfo info) {
        setText("This report refers to your gun, the ", "gunName", ".", info);
    }

    public void setText(String pre, String variableKey, String post, ClientInfo info) {
        String variable = info.lookup(variableKey);
        setParagraphText(pre, variable, post);
    }

    private void setParagraphText(String pre, String variable, String post) {
        text = pre + variable + post;
    }   
}

We’d like to get rid of those explicit set methods, which will push responsibility for setting the text and variables up in the Report - Page - Paragraph chain. We’re at a green bar (and checked in). We can refactor. Therefore, we can change this, in ReportPage:

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

… to this:

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

We’re still green and we can now remove setClientName from ReportParagraph. Still green. Now the same thing for fillInSummaryInfo():

   private void fillInSummaryInfo(ClientInfo info) {
        ReportParagraph gunPara = new ReportParagraph();
        gunPara.setGunName(info);
        paragraphMap.put("GunInformation", gunPara);
    }

… to:

   private void fillInSummaryInfo(ClientInfo info) {
        ReportParagraph gunPara = new ReportParagraph();
        gunPara.setText("This report refers to your gun, the ", "gunName", ".", info);
        paragraphMap.put("GunInformation", gunPara);
    }

So now we have ReportParagraph slimmed down and made “completely” general:

public class ReportParagraph {
    String text = "uninitialized";

    public String text() {
        return text;
    }

    public void setText(String pre, String variableKey, String post, ClientInfo info) {
        String variable = info.lookup(variableKey);
        setParagraphText(pre, variable, post);
    }

    private void setParagraphText(String pre, String variable, String post) {
        text = pre + variable + post;
    }   
}

Well, perfectly general as long as paragraphs just have one substitution. Which is all out tests need. Let’s do some inlining, to get:

public class ReportParagraph {
    String text = "uninitialized";

    public String text() {
        return text;
    }

    public void setText(String pre, String variableKey, String post, ClientInfo info) {
        text = pre + info.lookup(variableKey) + post;
    }
}

Pause for Reflection ...

We have added very little new code. A lookup method in clientInfo, supported by an oracleDatabase, um, HashMap. We’ve removed the paragraph type-specific code from ReportParagraph, by pushing the paragraph text, and the substitutions, up into ReportPage.

We have pushed “generality” downward, with a net reduction in code. We have pulled “specificity” upward, moving the specific notions of the boilerplate text, and the substitutable values, up toward the top of the program, where they can soon be extracted into external files for the customer (Chet) to manage as he wishes.

All of this has happened based on two specific cases. We wrote them, observed the duplication, and removed it. We have rewritten essentially nothing, only adding methods and replacing a few lines here and there. Everything has been essentially rote. In fact, we’ve done it while sitting in a coffee shop badgering Bill Tozier about why he dogears his cards.

A n y w a y … we have now moved the text up to the ReportPage. ReportPage contains duplication!!!

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;
    }

    public static ReportPage makeSummaryPage(ClientInfo info) {
        ReportPage summaryPage = new ReportPage();
        summaryPage.fillInSummaryInfo(info);
        return summaryPage;
    }

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

    private void fillInSummaryInfo(ClientInfo info) {
        ReportParagraph gunPara = new ReportParagraph();
        gunPara.setText("This report refers to your gun, the ", "gunName", ".", info);
        paragraphMap.put("GunInformation", gunPara);
    }
}

The two make methods look alike. The two fillIn methods also look alike: In their case I’ve highlighed all the equal bits … most of what isn’t equal is arbitrary and could be made equal. The rest of the variability is what amounts to user-provided paragraph content, or ClientInfo lookup keys.

All this duplication is a clear sign of an opportunity to create a method with the common bits. What shall we do?

We will extract a method that does all the “same” things in the fillIn methods, and call it from them, with the variable bits. Most likely, we’ll then do the same thing with the make methods, essentially extracting and inlining our way to generality. First the extract and usage. I don’t see a way to use the refactoring tool, so I’ll just copy and edit:

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;
    }

    public static ReportPage makeSummaryPage(ClientInfo info) {
        ReportPage summaryPage = new ReportPage();
        summaryPage.fillInSummaryInfo(info);
        return summaryPage;
    }

    private void fillInCoverInfo(ClientInfo info) {
        fillIn("ClientName","", "clientName", "", info);
    }

    private void fillInSummaryInfo(ClientInfo info) {
        fillIn("GunInformation", "This report refers to your gun, the ", "gunName", ".", info);
    }

    private void fillIn(String paragraphName, String pre, String key, String post, ClientInfo info) {
        ReportParagraph para = new ReportParagraph();
        para.setText(pre, key, post, info);
        paragraphMap.put(paragraphName, para);
    }
}

We see some duplication now in the make methods. And we have two fillIn methods that look just alike. Let’s inline some stuff:

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.fillIn("ClientName","", "clientName", "", info);
        return coverSheet;
    }

    public static ReportPage makeSummaryPage(ClientInfo info) {
        ReportPage summaryPage = new ReportPage();
        summaryPage.fillIn("GunInformation", "This report refers to your gun, the ", "gunName", ".", info);
        return summaryPage;
    }

    private void fillIn(String paragraphName, String pre, String key, String post, ClientInfo info) {
        ReportParagraph para = new ReportParagraph();
        para.setText(pre, key, post, info);
        paragraphMap.put(paragraphName, para);
    }
}

Digression1

It may seem to you that this incremental way of “discovering” the design is slow and tedious, but that’s because I’m explaining everything we say out loud, most of the things I think2, and pretty much every single keystroke. If you look at what really happened, we extracted a three-line method via cut / paste / edit, and then we called it from the old fillInX methods, and then we inlined those functions up into the makeX methods. It was just a moment’s work, despite the several minutes I had to spend writing about it and that you had to spend reading about it.

These changes are easy to make, and easy to make correctly. The code, at the end, contains less duplication, is smaller, and – we claim – better. We’ll talk about this further below.

End Digression, Back to Work

Now the two static make methods are full of duplication. Let’s extract it. First, just extract all those literal parameters to locals:

   public static ReportPage makeCoverSheet(ClientInfo info) {
        String pageName = "ClientName";
        String pre = "";
        String key = "clientName";
        String post = "";
        ReportPage newPage = new ReportPage();
        newPage.fillIn(pageName,pre, key, post, info);
        return newPage;
    }

    public static ReportPage makeSummaryPage(ClientInfo info) {
        String pageName = "GunInformation";
        String pre = "This report refers to your gun, the ";
        String key = "gunName";
        String post = ".";
        ReportPage newPage = new ReportPage();
        newPage.fillIn(pageName, pre, key, post, info);
        return newPage;
    }

This makes it clear how generic these two methods are. Then extract method on the blue bits, and voila:

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) {
        return makePage("ClientName", "", "clientName", "", info);
    }

    public static ReportPage makeSummaryPage(ClientInfo info) {
        return makePage("GunInformation", "This report refers to your gun, the ", "gunName", ".", info);
    }

    private static ReportPage makePage(String pageName, String pre, String key, String post, ClientInfo info) {
        ReportPage newPage = new ReportPage();
        newPage.fillIn(pageName,pre, key, post, info);
        return newPage;
    }

    private void fillIn(String paragraphName, String pre, String key, String post, ClientInfo info) {
        ReportParagraph para = new ReportParagraph();
        para.setText(pre, key, post, info);
        paragraphMap.put(paragraphName, para);
    }
}

Session Review

Let’s review. We started with a lot of duplication, and with the details of paragraphs going in at the very bottom of the code. Now, by smoothing the code, we are moving more general functionality downward, and specifics upward. Real soon now, we expect that the specifics will bubble far enough toward the top to be factored out into a file format or something similar.

This is turning out just as we expected, at least so far. Back in our article, Calling the Shot, we said that we were going to defer building a framework for doing this job, instead taking stories in the customer’s order. We predicted:

  • If we work on the framework, now, the customer won't feel the need for it and won't like the stories much;
  • If we work on the framework, now, we won't really know what we need, and we're likely to do too much, and too little, both at the same time;
  • Deferring the framework will get more useful work done;
  • We'll learn more and make fewer mistakes;
  • The cost of putting it in later won't be very large at all, because what will happen is that we'll be putting it in little by little.

How are we doing so far? We’re certainly learning what we need, and as we learn, we are moving in the direction of building for it. When we got a second kind of paragraph, we noticed the duplication, and reduced it. We did so in a way that moved the generalized functions down and the specifics up, which is what you do in a framework situation, put generality inside the framework, configuration outside.

We’re learning a lot, by actually doing, rather than speculating about what we might need in the framework. So far, putting in framework-like elements is just part of the everyday work of smoothing the code before putting it away.

Tiny Clues?

On the TDD list (yahoo testdrivendevelopment), Tony Tubbs asked:

3 or 4 articles ago, you point out the similarities between the Report and Page as far as how indexing is done. Once you point it out, I see it, but I didn't see it until then. I expect that I don't typically pick up similarities at this level. I'm wondering if this is something I should notice right off, if most would, and why it is that I didn't? What is it that makes this noticeable to you and I am assuming should to anyone in the role of reviewer? HashMaps are not uncommon really, and just noticing that two classes both make use of them doesn't seem like enough motivation to start wondering if you can factor that out into common code. Am I making sense here?

I think there are a few reasons why we see this:

  1. We practice this all the time, so we're starting to get good at it.
  2. We know we are working very simply but want generality. So we pay close attention to similar behavior, which engendners demand-driven generality as opposed to speculative generality.
  3. We talked about the overall structure of reports containing pages containing paragraphs. That made us think about the Composite pattern, which is one way of handling things which contain groups of things. That observation makes us sensitive to similarity.
  4. We try to do things the same way most times, unless we are explicitly trying to use a different way to find out what happens. So we essentially copied and pasted that code. Copy/paste is a clear indicator that there is duplication.

So there are reasons. In the end, it all comes down to practice.

Conclusions ... for now ...

For us, we are practiced at this. We know how to program the “old-fashioned” way, with up front design and coding for generality and all that. And, through practice, we know this way as well … and we prefer it strongly.

We speculate that as you practice the techniques we write about, yo’ll find yourself preferring them as well. We’ll only know if you try.

Stay tuned!


  1. This discussion brought to you courtesy of Kelly Anderson and others, who asked on the lists whether this approach isn't inefficient compared to just doing things right. Short answer: it might be, if (a) we could actually do things right, which in our case we can't; and (b) it didn't take a long time to do, which in our case it does; and (c) time debugging was short, which in our case it isn't.
  2. I imagine that Chet thinks also, but I draw that conclusion only because I am one of those people who imagine that everyone else is like them.