This week, I was introduced to Hopleaf, a craft beer bar in Chicago with 60 rotating taps. One of my favorite bars in Tucson, 1702, was similar. But every time I visited, I would face a dilemma: I usually only visit twice a month, and drink 3±1 beers each time. As a reformed economist, I insist on optimizing every decision I make, and with each beer I order, I face a trade-off between tasting a new beer of uncertain quality or choosing my favorite so far. If I want to maximize expected utility for each month’s visits, what is my best strategy? How many beers should I spend exploring for better ones, and how many should I spend enjoying my favorite?

Richard Feynman posed this problem in the context of finding the best dish at a new restaurant. (But beers are way more interesting). The solution makes sense (and come on, it’s Richard Feynman), but I wanted to check his work with a simulation.

You can find the results (and the rest of this post) in this iPython notebook. There’s much more there, including an interesting result!

Martin Fowler identifies long methods as another common code smell, fixable by breaking one long method into several smaller ones and composing them together. Since Clean Code emphasized writing short, meaningful methods, I had to look a bit to find one. But I’m not very happy with the board coordinate constructor:

1
2
3
4
5
6
7
8
9
10
11
public UniversalBoardCoordinate(String locationPhrase) throws InvalidCoordinateException {
        String noParens = locationPhrase.replace('(', ' ').replace(')', ' ');
        String[] coordinates = noParens.split(",");

        if (coordinates.length != 2) {
            throw new InvalidCoordinateException("That's not a valid board location.");
        }

        row = Integer.parseInt(coordinates[0].trim());
        column = Integer.parseInt(coordinates[1].trim());
    }

This is an easy refactor: think about what each line of code does, group the related ones in their own methods, and replace them. In fact, I’d already separated each group with a line break. The first two trim the input string and split it in two:

1
2
3
4
5
6
private Integer[] parseString(String locationPhrase) throws InvalidCoordinateException {
        String noParens = locationPhrase.replace('(', ' ').replace(')', ' ');
        String[] coordinates = noParens.split(",");
        checkValidity(coordinates);
        return parseCoordinates(coordinates);
}

The next two check whether the result is valid:

1
2
3
4
5
private void checkValidity(String[] coordinates) throws InvalidCoordinateException {
        if (coordinates.length != 2) {
            throw new InvalidCoordinateException("That's not a valid board location.");
        }
}
1
2
3
4
5
And the last two convert the strings to integers:
private Integer[] parseCoordinates(String[] coordinates) {
        return new Integer[] { Integer.parseInt(coordinates[0].trim()),
                               Integer.parseInt(coordinates[1].trim()) };
}

Now that the details of string manipulation are hidden in helper methods, the complicated constructor looks trivial:

1
2
3
4
5
6
public UniversalBoardCoordinate(String locationPhrase) throws InvalidCoordinateException {
        Integer[] orderedPair = parseString(locationPhrase);

        row = orderedPair[0];
        column = orderedPair[1];
}

Before finishing up the first version of my terminal view game, I moved strings like the welcome message and player prompts to a properties file. This requires reading and storing the strings before using them in the view, but has two big advantages. First, tests no longer break when I edit a string (at least, as long as my tests are using the same properties). Second, if I wanted to translate my program into another language, it’s as easy as swapping out the properties file.

Unfortunately, loading strings got ugly fast. Here’s an example from the GameController class:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
public class GameController implements Controller {

    private String welcome;
    private String divider;
    private String yourMove;
    private String yourMoveThreeSquares;
    private String playAgain;
    private String gameOverDraw;
    private String gameOverWin;
    private String xWins;
    private String oWins;
    private String choosePlayerOne;
    private String choosePlayerTwo;
    private String boardSize;

    public GameController(View gameView) {
        loadViewStrings();
        view = gameView;
        board = new GameBoard();
    }

    private void loadViewStrings() {
        Properties viewStrings = new Properties();
        try {
            viewStrings.load(getClass().getResourceAsStream("/viewstrings.properties"));
        } catch (IOException e) {
            System.out.println(e);
        }

        welcome = viewStrings.getProperty("welcome");
        divider = viewStrings.getProperty("divider");
        yourMove = viewStrings.getProperty("yourmove");
        yourMoveThreeSquares = viewStrings.getProperty("yourmovethreesquares");
        playAgain = viewStrings.getProperty("playagain");
        gameOverDraw = viewStrings.getProperty("gameoverdraw");
        gameOverWin = viewStrings.getProperty("gameoverwin");
        xWins = viewStrings.getProperty("xwins");
        oWins = viewStrings.getProperty("owins");
        choosePlayerOne = viewStrings.getProperty("chooseplayerone");
        choosePlayerTwo = viewStrings.getProperty("chooseplayertwo");
        boardSize = viewStrings.getProperty("boardsize");

    }
 }
 

  It gets worse. When I started writing my Swing view, I needed to ignore certain strings, like those that prompt for keyboard input. A good idea: avoid creating a brand new SwingController class, but somehow filter out unneeded strings. A bad idea: do it by checking and ignoring certain strings from the controller. This is logic that really shouldn’t be in the view, implemented in a very fragile way. In fact, it undoes all the abstraction of the properties file—as soon as a string changes, displayMessage() will probably break. Plus, there’s plenty of duplicated code.

 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
 public class SwingView extends JFrame implements View {

    private String divider;
    private String playAgain;
    private String choosePlayerOne;
    private String choosePlayerTwo;
    private String boardSize;

    private Set<String> ignoreThese = new HashSet<String>();

    public SwingView() {
        loadViewStrings();
    }

    private void loadViewStrings() {
        Properties viewStrings = new Properties();
        try {
            viewStrings.load(getClass().getResourceAsStream("/viewstrings.properties"));
        } catch (IOException e) {
            System.out.println(e);
        }

        divider = viewStrings.getProperty("divider");
        playAgain = viewStrings.getProperty("playagain");
        choosePlayerOne = viewStrings.getProperty("chooseplayerone");
        choosePlayerTwo = viewStrings.getProperty("chooseplayertwo");
        boardSize = viewStrings.getProperty("boardsize");

        ignoreThese.add(divider);
        ignoreThese.add(playAgain);
        ignoreThese.add(choosePlayerOne);
        ignoreThese.add(choosePlayerTwo);
        ignoreThese.add(boardSize);
    }

    public void displayMessage(String message) {
        if (message.contains("move")) {
            int endStart = message.indexOf("player") + 6;
            String ending = message.substring(endStart);
            message = "Your move, player" + ending;
        } else if (ignoreThese.contains(message)) {
            return;
        }
        JLabel messageLabel = messagePanel.getLabel();
        messageLabel.setText(message);
   }   
 }
 

   This is similar, though not identical to Fowler’s “Refused bequest,” where a subclass inherits lots of methods and then ignores them. Here’ I’m loading lots of strings, then ignoring them.    Refused bequest is fixed with a refactor called “Replace inheritance with delegation:” put the superclass in a field on the old subclass, remove the inheritance, and simply delegate to the superclass methods when needed. Here, I haven’t even shared code through inheritance, but by good old copy-and-paste (so it’s also part of the duplicated code stink parade).    The cleanup strategy here is similar too: create a separate class to handle loading properties, and use it in place of the duplicated methods. I’ll start by creating a StringLoader class that includes the duplicated fields and methods:    

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
 public class StringLoader {
    String welcome;
    String divider;
    String yourMove;
    String yourMoveThreeSquares;
    String playAgain;
    String gameOverDraw;
    String gameOverWin;
    String xWins;
    String oWins;
    String choosePlayerOne;
    String choosePlayerTwo;
    String boardSize;

    public StringLoader() {
    }

    void loadViewStrings() {
        Properties viewStrings = new Properties();
        try {
            viewStrings.load(null.getClass().getResourceAsStream("/viewstrings.properties"));
        } catch (IOException e) {
            System.out.println(e);
        }

        welcome = viewStrings.getProperty("welcome");
        divider = viewStrings.getProperty("divider");
        yourMove = viewStrings.getProperty("yourmove");
        yourMoveThreeSquares = viewStrings.getProperty("yourmovethreesquares");
        playAgain = viewStrings.getProperty("playagain");
        gameOverDraw = viewStrings.getProperty("gameoverdraw");
        gameOverWin = viewStrings.getProperty("gameoverwin");
        xWins = viewStrings.getProperty("xwins");
        oWins = viewStrings.getProperty("owins");
        choosePlayerOne = viewStrings.getProperty("chooseplayerone");
        choosePlayerTwo = viewStrings.getProperty("chooseplayertwo");
        boardSize = viewStrings.getProperty("boardsize");

    }
}

Instead of all these fields, storing strings in a map is much cleaner:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
public class StringLoader {
    private HashMap<String, String> viewStrings = new HashMap<String, String>();

    public StringLoader() {
    }

    void loadViewStrings() {
        Properties viewStrings = new Properties();
        try {
            viewStrings.load(getClass().getResourceAsStream("/viewstrings.properties"));
        } catch (IOException e) {
            System.out.println(e);
        }

        welcome = viewStrings.getProperty("welcome");
        divider = viewStrings.getProperty("divider");
        yourMove = viewStrings.getProperty("yourmove");
        yourMoveThreeSquares = viewStrings.getProperty("yourmovethreesquares");
        playAgain = viewStrings.getProperty("playagain");
        gameOverDraw = viewStrings.getProperty("gameoverdraw");
        gameOverWin = viewStrings.getProperty("gameoverwin");
        xWins = viewStrings.getProperty("xwins");
        oWins = viewStrings.getProperty("owins");
        choosePlayerOne = viewStrings.getProperty("chooseplayerone");
        choosePlayerTwo = viewStrings.getProperty("chooseplayertwo");
        boardSize = viewStrings.getProperty("boardsize");

    }
}

The repeated calls to viewStrings.getProperty() are still pretty ugly and very difficult to read. One solution is to extract a load() method, then iterate over an array of the property names:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
public class StringLoader {
    private HashMap<String, String> viewStrings = new HashMap<String, String>();
    private Properties viewStringProperties = new Properties();

    public StringLoader() {
        loadViewStrings();
    }

    private void load(String propertyName) {
        String propertyString = viewStringProperties.getProperty(propertyName);
        viewStrings.put(propertyName, propertyString)
    }

    private void loadViewStrings() {
        try {
            viewStringProperties.load(getClass().getResourceAsStream("/viewstrings.properties"));
        } catch (IOException exception) {
            exception.printStackTrace();
        }

        String[] properties = { "welcome",
                                "divider",
                                "yourmove",
                                "yourmovethreesquares",
                                "playagain",
                                "gameoverdraw",
                                "gameoverwin",
                                "xwins",
                                "owins",
                                "chooseplayerone",
                                "chooseplayertwo",
                                "boardsize"};

        for (String property : properties) {
            load(property);
        }

    }
}

Finally, let’s replace the hardcoded filepath by passing a path to the view constructor (and on to the loadViewStrings() method:

1
2
3
public StringLoader(String filepath) {
      loadViewStrings(filepath);
}

Now, reading in strings from the properties file looks like this, instead of the forty-line monster we started with:

1
2
3
4
public class GameController implements Controller {
    private HashMap<String, String> viewStrings =
      new StringLoader().getViewStrings("/viewstrings.properties");
}

To use a string, I can just get it from the map:

1
viewStrings.get("welcome");

This requires trading off a little clarity, but on balance it’s much cleaner. Best of all, now I can use properties as they were intended. Preventing my view from displaying certain strings just requires creating a new properties file and removing the content from the unneeded messages—another win for decoupling and abstraction.

I’ve spent the last couple weeks working through Martin Fowler’s Refactoring, which includes a taxonomy of useful solutions to common code smells. It’s an excellent book, but I found it hard to recognize and remember some of the patterns without digging into my own code. Over the next few posts, I’ll describe some of the smells and refactors I found in my Tic Tac Toe game (as much to commit them to my own memory as to share them with others). First up, the most common code smell of all: duplication.

Despite using a superclass to store static methods and data used by all my tests, I wasn’t using inheritance to create universal setup and teardown methods. Although many of my tests used a similar pattern to capture output, none were quite alike, and all of them were pretty messy. Here’s an example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
@RunWith(JUnit4.class)
public class GameControllerTest {

    private MockTerminalView view = new MockTerminalView();
    private GameController controller = new GameController(view);

    private final PrintStream stdout = System.out;
    private final ByteArrayOutputStream output = new ByteArrayOutputStream();
    private PrintStream outputStream;
    private OutputRecorder outputRecorder;
  
    @Before
    public void setUp() throws Exception {
        loadViewStrings();
        outputStream = new PrintStream(output, true, "UTF-8");
        outputRecorder = new OutputRecorder(output, true, "UTF-8");
        Player playerOne = new HumanPlayer(X);
        Player playerTwo = new MinimaxPlayer(O);
        controller.setPlayerOne(playerOne);
        controller.setPlayerTwo(playerTwo);
    }

    @Test
    public void controllerShouldStartNewGame() {
        System.setOut(outputRecorder);

        controller.newGame();

        assertEquals(welcome, outputRecorder.popFirstOutput());
        assertEquals(divider, outputRecorder.popFirstOutput());

        System.setOut(stdout);
    }

    @Test(expected = GameOverException.class)
    public void controllerShouldEndGameOnRestartIfInputIsNo() throws GameOverException {
        System.setOut(outputStream);

        view.enqueueInput("n");

        controller.restartGame();
        assertEquals(playAgain, output.toString());

        System.setOut(stdout);
    }
} 

These tests use both a plain PrintStream and my custom OutputRecorder, though they don’t really need to. Some setup is done in the fields, and some during setUp(), though there’s no clear reason why. And the same lines setting up and tearing down the recorder are repeated across lots of tests. The smell here is duplicated code, which Fowler calls “number one in the stink parade.” To solve it, I’ll start by extracting a method, and then extracting a superclass.

First, I’ll create an empty TicTacToeTest class with empty setup and teardown methods.

1
2
3
4
5
6
7
8
9
10
11
@RunWith(JUnit4.class)
public class TicTacToeTest {

  @Before
  public void setUp() {
  }

  @After
  public void cleanUp() {
  }
}

Now, extract a setUpRecorder() method including all the setup-related lines:

1
2
3
4
5
6
7
8
            private OutputRecorder recorder;

            private void setUpRecorder() throws UnsupportedEncodingException {
                    PrintStream stdout = System.out;
                    ByteArrayOutputStream output = new ByteArrayOutputStream();
                    recorder = new OutputRecorder(output, true, "UTF-8");
            }

Then, a startRecorder() method to replace calls to System.setOut():

1
2
3
4
            private void startRecorder() {
              System.setOut(recorder);
              }

This method might seem short, but I find startRecorder() much easier to read. Now that these methods are implemented, I can plug them into the tests:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
              @Before
              public void setUp() throws Exception {
                  loadViewStrings();
                  
                  setUpRecorder();
                  
                  Player playerOne = new HumanPlayer(X);
                  Player playerTwo = new MinimaxPlayer(O);
                  controller.setPlayerOne(playerOne);
                  controller.setPlayerTwo(playerTwo);
              }

              @Test
              public void controllerShouldStartNewGame() {
                  startRecorder();

                      controller.newGame();

                      assertEquals(welcome, outputRecorder.popFirstOutput());
                      assertEquals(divider, outputRecorder.popFirstOutput());

                      System.setOut(stdout);
                  }

The last line of the second test was meant as a mini-teardown, to reset stdout before the next test. I wrote it before I understood JUnit execution, in which each test runs in its own environment. It survived around 80 commits, but it doesn’t actually do anything, so I can safely delete it. (I know because none of the tests fail afterwards). In fact, this whole refactor should be possible while keeping the tests green.

After searching for usages of the old pattern and replacing them with the new methods, there’s just one thing left: pull up the new methods to a Test superclass. Here’s the result:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
                  public class TicTacToeTest {

                      protected OutputRecorder recorder;

                      protected void setUpRecorder() throws UnsupportedEncodingException {
                          ByteArrayOutputStream output = new ByteArrayOutputStream();
                          recorder = new OutputRecorder(output, true, "UTF-8");
                      }

                      protected void startRecorder() {
                          System.setOut(recorder);
                      }

                      @Before
                      public void recorderSetUp() throws UnsupportedEncodingException {
                          setUpRecorder();
                      }

                  }

This looks like a simple refactor, but my tests are now much cleaner. More important, if I need to make a change to the OutputRecorder class, it will propagate through all tests.

One of last week’s projects was setting up Travis CI, a hosted continuous integration service that’s free for open source projects. Even if you haven’t heard about Travis (as I hadn’t), you’ve probably already seen the service on Github—it’s responsible for the green and red build status icons at the top of many project READMEs.

After every push to a remote repo, Travis builds your project, runs the test suite, and sends off a notification with the results. Once it’s configured, it’s a great way to ensure that even if your project is unfinished, the published code will work correctly for anyone who pulls the repo.

The official Travis docs are close to comprehensive, but I ran into a few catches getting everything set up (and have an inbox full of failed build notifications to prove it). Hopefully these solutions will rescue someone else from a frustrating afternoon.

Resolving Ivy dependencies

My Tic Tac Toe project uses Ivy to manage build dependencies, and my ant build script includes a standard snippet that downloads Ivy if it’s not available locally:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
<property name="ivy.install.version" value="2.1.0-rc2" />
  <condition property="ivy.home" value="${env.IVY_HOME}">
    <isset property="env.IVY_HOME" />
  </condition>
<property name="ivy.home" value="${user.home}/.ant" />
<property name="ivy.jar.dir" value="${ivy.home}/lib" />
<property name="ivy.jar.file" value="${ivy.jar.dir}/ivy.jar" />

<target name="download-ivy" unless="offline">

  <mkdir dir="${ivy.jar.dir}"/>
  <!-- download Ivy from web site so that it can be used even without --
    -- any special installation -->
  <get src="http://repo2.maven.org/maven2/org/apache/ivy/ivy/${ivy.install.version}/ivy-${ivy.install.version}.jar"
       dest="${ivy.jar.file}"
       usetimestamp="true"/>
</target>

<target name="init-ivy"
        depends="download-ivy">
<!-- try to load ivy here from ivy home, in case the user has not --
  -- already dropped it into ant's lib dir (note that the latter copy --
  -- will always take precedence). We will not fail as long as local --
  -- lib dir exists (it may be empty) and ivy is in at least one of --
  -- ant's lib dir or the local lib dir. -->
  <path id="ivy.lib.path">
    <fileset dir="${ivy.jar.dir}"
             includes="*.jar"/>

  </path>
  <taskdef resource="org/apache/ivy/ant/antlib.xml"
           uri="antlib:org.apache.ivy.ant"
           classpathref="ivy.lib.path"/>
</target>

If your build and test targets are set to depend on the init-ivy task, the script will ensure that Ivy resolves itself and the project’s other dependencies correctly. Telling Travis how to build and test a project is as simple as writing a few lines of YAML. For example:

1
2
3
4
5
6
language: java
install: ant resolve
jdk:
  - oraclejdk7
  - openjdk7
  - openjdk6

This tells Travis the project language (which comes with its own default settings), points to ant resolve as the command necessary to resolve dependencies, and describes the target Java versions to test against. But my builds failed with the following mysterious error:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
$ ant resolve
Buildfile: /home/travis/build/ecmendenhall/Java-TTT/build.xml

  [mkdir] Created dir: /home/travis/build/ecmendenhall/Java-TTT/lib

check-ivy:
  [echo] Checking for Ivy .jar in local directories.

bootstrap-ivy:
  [echo] Bootstrapping Ivy installation.
  [mkdir] Created dir: /home/travis/.ant/lib
  [get] Getting: http://search.maven.org/remotecontent?filepath=org/apache/ivy/ivy/2.3.0/ivy-2.3.0.jar
  [get] To: /home/travis/.ant/lib/ivy.jar

resolve:
  [echo] Resolving project dependencies.

BUILD FAILED
/home/travis/build/ecmendenhall/Java-TTT/build.xml:52: Problem: failed
to create task or type antlib:org.apache.ivy.ant:retrieve
Cause: The name is undefined.
Action: Check the spelling.
Action: Check that any custom tasks/types have been declared.
Action: Check that any <presetdef>/<macrodef> declarations have taken
        place.

        No types or tasks have been defined in this namespace yet
        This appears to be an antlib declaration.
        Action: Check that the implementing library exists in one of:
            -/usr/share/ant/lib
            -/home/travis/.ant/lib
            -a directory added on the command line with the -lib argument

Even though ant was correctly downloading an ivy jarfile, the build script failed to find it. The answers to my Stack Overflow question explained that this was related to the order in which jars are loaded to the Java classpath. The easiest solution is to run ant twice, once to download ivy, and again to build and test the project.

Once the source of this error was cleared up, the solution was simple. Adding a before_install script to my Travis build ensured that ivy would be downloaded before trying to build the project. This meant adding just one line to my .travis.yml:

1
2
3
4
5
6
7
language: java
before_install: ant init-ivy
install: ant resolve
jdk:
  - oraclejdk7
  - openjdk7
  - openjdk6

There are a number of other options to run scripts at different times in the Travis lifecycle. Check out the rest of the docs here.

Headless testing with xfvb

My Travis builds worked nicely until I started adding a Swing view to my Tic Tac Toe project. Although the tests would run and pass locally, Travis threw a java.awt.HeadlessException as soon as it tried to run the test suite. The Travis docs explain using a tool called xvfb (X Virtual Framebuffer) to simulate a windowing system and run headless tests, but warn that “you need to tell your testing tool process” exactly how to use it.

Exactly how to do this wasn’t clear, but after fiddling with JVM startup arguments and trying to shell out from inside ant, I discovered that the solution was dead simple: just add the recommended arguments to .travis.yml, and ant and JUnit will take care of the rest. Here’s my final .travis.yml, including the solutions to both problems.

1
2
3
4
5
6
7
8
9
10
language: java
before_install:
  - ant init-ivy
  - "export DISPLAY=:99.0"
  - "sh -e /etc/init.d/xvfb start"
install: ant resolve
jdk:
  - oraclejdk7
  - openjdk7
  - openjdk6

“It is a revelation to compare Menard’s Don Quixote with Cervantes’. The latter, for example, wrote (part one, chapter nine): ‘…truth, whose mother is history, rival of time, depository of deeds, witness of the past, exemplar and adviser to the present, and the future’s counselor.’ Written in the seventeenth century, written by the ‘lay genius’ Cervantes, this enumeration is a mere rhetorical praise of history. Menard, on the other hand, writes: ‘…truth, whose mother is history, rival of time, depository of deeds, witness of the past, exemplar and adviser to the present, and the future’s counselor.’”

–From Pierre Menard, Author of the Quixote

Macros are hard, but one of the most helpful exercises I’ve found in my limited macro writing experience is practicing by recreating some of the core macros I already know and love, like or, when-let, and ->.

Using speclj greatly simplifies this exercise, and writing macro specs often test my understanding better than writing the implementations themselves. Here’s an example spec for the threading macro:

1
2
3
4
5
6
7
8
9
10
11
12
13
(describe "-> macro"
  (it "should expand into the code below"
    (let [macro-form '(->-macro 1 (+ 2) (* 3))]

      (should= '(macro-challenges.core/->-macro
                  (macro-challenges.core/->-macro 1 (+ 2)) (* 3))
                (macroexpand-1 macro-form))

      (should= '(* (macro-challenges.core/->-macro 1 (+ 2)) 3)
                (macroexpand macro-form))

      (should= '(* (+ 1 2) 3)
                (macroexpand-all macro-form)))))

The functions macroexpand, macroexpand-1, and macroexpand-all come in very handy. Macroexpand-1 returns the “first” expansion of a macro form (macros within macros won’t be expanded). Macroexpand calls macroexpand-1 until the expansion is no longer a macro form.

In the above example, the first expansion of ->-macro, my threading macro replacement, returns a form that starts with another call to ->-macro. (I was surprised to find out that this is how the threading macro works under the hood). Macroexpand expands into a list until the first item is *, which is not a macro.

When it’s macros all the way down, macroexpand-all (technically clojure.walk/macroexpand-all) recursively expands all macros in a given form, resulting in the much simpler expression (* (+ 1 2) 3) in the example above. These functions are all hugely helpful for writing macros and their associated tests.

Here’s my recreation of ->, which passed the spec:

1
2
3
4
5
(defmacro ->-macro
  ([arg] arg)
  ([arg first-form] `(~(first first-form) ~arg ~@(rest first-form)))
  ([arg first-form & more-forms]
     `(->-macro (->-macro ~arg ~first-form) ~@more-forms)))

And bears a pretty strong resemblance to the original source:

1
2
3
4
5
6
7
8
9
10
11
(defmacro ->
  "Threads the expr through the forms. Inserts x as the
  second item in the first form, making a list of it if it is not a
  list already. If there are more forms, inserts the first form as the
  second item in second form, etc."
  {:added "1.0"}
  ([x] x)
  ([x form] (if (seq? form)
              (with-meta `(~(first form) ~x ~@(next form)) (meta form))
              (list form x)))
  ([x form & more] `(-> (-> ~x ~form) ~@more)))

Some macros are pretty easy to reconstruct (but check their documentation to make sure you really understand how they handle different arguments). If you’re well and truly stuck, it’s always possible to check out the original code for inspiration.

The specs and solutions I’ve written so far (mostly low-hanging fruit) are all available on my Github, if you’d like to have a hand at reconstructing Clojure yourself.

Clojure may be a new language, but Lisp has a long history. Translating Scheme and Common Lisp classics into Clojure is always an interesting exercise, and often illuminates the differences and comparative advantages of various Lisp-y languages. (For more Lisp-to-Clojure resources see this list. Or, if you’d like to try porting some Scheme, consider helping translate SICP).

This weekend, I spent some time with Paul Graham’s classic On Lisp. In Chapter 6, Graham shows how to use closures to model nodes in a network, representing a 20 questions game as a self-traversing binary tree. Here’s his original code, and my best attempts at Clojure translations.

The most obvious model for a set of connected nodes is a nested data structure, like a map of maps. In Common Lisp, Graham uses a mutable hashmap of structured types, each pointing to their neighbors in the network:

1
2
3
4
5
6
7
8
9
10
(defstruct node contents yes no)

(defvar *nodes* (make-hash-table))

(defun defnode (name conts &optional yes no)

(setf (gethash name *nodes*)
  (make-node :contents conts
    :yes yes
    :no no)))

A simple map seems like a sufficient replacement in Clojure. A single node looks like this:

1
{:people {:contents "Is the person a man?", :yes :male, :no :female}}

And the full tree looks like the following. Each node’s :yes or :no keyword points to the next node in the tree:

1
2
3
4
5
6
{:penny {:contents "Abraham Lincoln.", :yes nil, :no nil},
 :coin  {:contents "Is the coin a penny?", :yes :penny, :no :other-coin},
 :USA   {:contents "Is he on a coin?", :yes :coin, :no :no-coin},
 :dead  {:contents "Was he from the USA?", :yes :USA, :no :elsewhere},
 :male  {:contents "Is he living?", :yes :live, :no :dead},
 :people {:contents "Is the person a man?", :yes :male, :no :female}}

Here’s my first draft for defining nodes in Clojure. Since the Common Lisp version used a mutable variable, I used a Clojure atom to store network state:

1
2
3
4
5
6
7
8
9
10
11
12
(def nodes (atom {}))

(defn defnode [name contents & [yes no]]
  (swap! nodes assoc name {:contents contents :yes yes :no no}))

(defn make-nodes []
  (defnode :people "Is the person a man?" :male :female)
  (defnode :male "Is he living?" :live :dead)
  (defnode :dead "Was he from the USA?" :USA :elsewhere)
  (defnode :USA "Is he on a coin?" :coin :no-coin)
  (defnode :coin "Is the coin a penny?" :penny :other-coin)
  (defnode :penny "Abraham Lincoln."))

Traversing the network is simple: get a node, print the associated question, prompt for input, get the next node, and repeat. Here’s the original Common Lisp:

1
2
3
4
5
6
7
8
(defun run-node (name)
  (let ((n (gethash name *nodes*)))
    (cond ((node-yes n)
           (format t "~A~%>> " (node-contents n))
           (case (read)
             (yes (run-node (node-yes n)))
             (t (run-node (node-no n)))))
          (t (node-contents n)))))

And here’s an equivalent in Clojure:

1
2
3
4
5
6
7
8
9
10
11
12
(defn run-node [name]
  (let [node     (@nodes name)
        contents (node :contents)
        yes      (node :yes)
        no       (node :no)]
    (if yes
      (do
        (println contents)
         (if (= "yes" (read-line))
           (run-node yes)
           (run-node no)))
      (println contents))))

Of course, there’s no reason to bother swapping and dereferencing an atom as long as the tree won’t need to change at runtime. This immutable version works just as well:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
(defn defnode [nodes name contents & [yes no]]
  (assoc nodes name {:contents contents :yes yes :no no}))

(defn make-nodes []
  (-> {}
    (defnode :people "Is the person a man?" :male :female)
    (defnode :male "Is he living?" :live :dead)
    (defnode :dead "Was he from the USA?" :USA :elsewhere)
    (defnode :USA "Is he on a coin?" :coin :no-coin)
    (defnode :coin "Is the coin a penny?" :penny :other-coin)
    (defnode :penny "Abraham Lincoln.")))

(def nodes (make-nodes))

(defn run-node [nodes name]
  (let [node     (nodes name)
        contents (node :contents)
        yes      (node :yes)
        no       (node :no)]
    (if yes
      (do
        (println contents)
         (if (= "yes" (read-line))
           (run-node nodes yes)
           (run-node nodes no)))
      (println contents))))

Using a closure rolls the data structure and traversal code into one, by associating the yes and no fields with anonymous functions that handle the same logic as run-node. Here’s Graham’s CL version:

1
2
3
4
5
6
7
8
9
(defun defnode (name conts &optional yes no)
  (setf (gethash name *nodes*)
        (if yes
          #’(lambda ()
              (format t "~A~%>> " conts)
              (case (read)
                (yes (funcall (gethash yes *nodes*)))
                (t (funcall (gethash no *nodes*)))))
          #’(lambda () conts))))

And here’s mine in Clojure. The double-parens around (@nodes yes) and (@nodes no) call the anonymous function, instead of just returning it.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
(def nodes (atom {}))

(defn defclosure [name contents & [yes no]]
  (swap! nodes assoc name
         (if yes
           (fn []
             (println contents)
             (if (= "yes" (read-line))
               ((@nodes yes))
               ((@nodes no))))
           (fn []
             (println contents)))))

(defn make-nodes []
  (defclosure :people "Is the person a man?" :male :female)
  (defclosure :male "Is he living?" :live :dead)
  (defclosure :dead "Was he from the USA?" :USA :elsewhere)
  (defclosure :USA "Is he on a coin?" :coin :no-coin)
  (defclosure :coin "Is the coin a penny?" :penny :other-coin)
  (defclosure :penny "Abraham Lincoln."))

Now, traversing the tree is as simple as calling:

1
((nodes :people))

And watching the tree traverse itself. You can find my code from this post here. For more on closures in Lisp, check out the rest of Chapter 6.

One more recipe for the TDD cookbook before I move on to more interesting things. You might have noticed a new GameOverException and a try-catch block in the final example of my last post.

Before adding a game over exception, methods that checked for final game states directly called System.exit(). Testing this proved difficult, and I ended up importing a third-party library of extra JUnit rules. This week, I refactored them to throw a custom exception:

TerminalView.java
1
2
3
4
5
6
7
public class TerminalView implements View {

    public void endGame() throws GameOverException {
        throw new GameOverException("Game over.");
    }

}

This exception will bubble up until it’s caught in Main, which calls System.exit() on behalf of any method that throws a GameOverException.

Main.java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
public class Main {

    public static void main(String[] args) {
        try {

            View view = new TerminalView();
            GameController controller = new GameController(view);

            controller.newGame();
            controller.setUp();
            controller.startGame();
        } catch (GameOverException e) {
            System.exit(0);
        }
    }
}

This is not only a better practice, but also provides a much better way to test methods that might detect a completed game—just add a try/catch block to the tests!

Exceptions have come in handy elsewhere in my tests, too. Once the controller starts a game, there’s nothing to break the back-and-forth game loop but a win, draw, or error. Figuring out how to test game states without getting stuck in an infinite loop or loading up entire games was a challenge. “If only there were some special syntax for breaking normal control flow in special situations,” I wondered to myself more times than I’d like to admit. Well, duh—use exceptions! My mock view objects throw a NoSuchElementException when their input queue runs empty. Catching this exception breaks the normal game flow and allows me to access game state as soon as the fake input I’m interested in has been sent to the controller. Here’s an example:

GameControllerTest.java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
@Test
public void controllerShouldPassErrorMessageToViewOnInvalidInput()throws GameOverException {
    view.enqueueInput("invalid phrase");

    controller.newGame();

    System.setOut(outputRecorder);

    try {
        controller.playRound();
    } catch (NoSuchElementException e) {

        outputRecorder.discardFirstNStrings(1);
        String output = outputRecorder.popFirstOutput();

        assertEquals("That's not a valid board location.", output);

    }

    System.setOut(stdout);
    view.clearInput();
}

Normally, Controller.playRound() will continue querying players for moves until the game ends. But once this test catches the empty queue exception, it tests against the expected output, which should show an error message. Exceptions have proved extremely handy so far—as long as I remember that they’re in my control flow toolbox, too.

Redirecting stdout to a new PrintStream is an easy way to test simple console output in Java. But as my Tic-Tac-Toe game has grown more complex, the tests I wrote using this pattern have started to stink. There’s a lot of duplicated code (create the stream, redirect, tear down with each test), each test uses a hand-constructed string filled with finicky newline characters, and tests are prone to break when unrelated view components change the way they print to the screen. Inspired by my input queue, I created an OutputRecorder class that extends PrintStream and captures output string by string for later playback:

OutputRecorder.java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
public class OutputRecorder extends PrintStream {
    private Deque<String> outputStack;

      public OutputRecorder(OutputStream outputStream, boolean b,
      String s) throws UnsupportedEncodingException {
          super(outputStream, b, s);
          outputStack = new LinkedList<String>();
        }

        private void catchOutput(String output) {
            outputStack.addFirst(output);
        }

        public String popLastOutput() {
            return outputStack.removeFirst();
        }

        public String popFirstOutput() {
            return outputStack.removeLast();
        }


        public String peekLastOutput() {
            return outputStack.peekFirst();
        }

        public String peekFirstOutput() {
            return outputStack.peekLast();
        }

        public void discardLastNStrings(int n) {
            for (int i=0; i < n; i++) {
                popLastOutput();
            }
       }

       public void discardFirstNStrings(int n) {
            for (int i=0; i < n; i++) {
                popFirstOutput();
            }
       }

       public void replayAllForwards() {
            String output = popFirstOutput();
            int i = 0;
            while (output != null) {
                System.out.println("Element" + i + ":");
                System.out.println(output);
                i += 1;
                output = popFirstOutput();
            }
       }

       public void replayAllBackwards() {
            String output = popLastOutput();
            int i = outputStack.size();
            while (output != null) {
                System.out.println("Element" + i + ":");
                System.out.println(output);
                i -= 1;
                output = popLastOutput();
            }
       }

       @Override
       public void println(String output) {
            catchOutput(output);
       }

       @Override
       public void print(String output) {
            catchOutput(output);
       }

}

Since the recorder stores strings in a deque, it’s easy to replay output in forward or reverse order. Now a test like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20

@Test
public void controllerShouldPassErrorMessageToViewOnInvalidMove() {
    view.pushInput("middle center");
    view.pushInput("middle center");

    exit.expectSystemExitWithStatus(2);
    controller.newGame();

    System.setOut(outputStream);

    controller.playRound();
    String expected = yourMove + "\n" + xInCenter.toString() + "\n";

    assertEquals(expected,
    output.toString());

    System.setOut(stdout);
    view.clearInput();
}

Become a little friendlier…

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
@Test
public void controllerShouldPassErrorMessageToViewOnInvalidMove() throws GameOverException {
    view.enqueueInput("middle center");
    view.enqueueInput("middle center");

    controller.newGame();

    System.setOut(outputRecorder);

    try {
        controller.playRound();
    } catch (NoSuchElementException e) {

          outputRecorder.discardFirstNStrings(4);
          String output = outputRecorder.popFirstOutput();

          assertEquals("Square is already full.", output);
    }
    System.setOut(stdout);
    view.clearInput();
}

The utility might not be immediately obvious, but capturing output string by string has already put an end to tracking down small differences between expected and actual output that come from an extra space or misplaced newline.

Over the course of my Tic Tac Toe project, I’ve needed to test against user input several times. As a newcomer to mock object patterns, coming up with good solutions to these testing dilemmas has been one of my biggest challenges.

There are plenty of heavy-duty tools for mocks and fakes in Java, but I’d like to stick with my own solutions as long as possible, since writing them myself has been enlightening.

Here’s a solution I came up with today to simulate full Tic Tac Toe games with two human players: a mock View object that returns fake input from a queue. By pushing mock input onto the queue during test setup, I can configure games in advance and replay them later. Here’s the very simple mock View object:

MockTerminalView.java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
import java.util.NoSuchElementException;
import java.util.Queue;
import java.util.concurrent.LinkedBlockingQueue;

public class MockTerminalView extends TerminalView {

    private Queue<String> inputQ = new LinkedBlockingQueue<String>();

    public void enqueueInput(String fakeInput) {
        inputQ.add(fakeInput);
    }

    public void clearInput() {
        inputQ.clear();
    }

    @Override
    public String getInput() {
        try {
            return inputQ.remove();
        } catch (NoSuchElementException e) {
            return "";
        }
    }
}

And here’s an example test that plays through an entire game and exits when Player 2 wins (comments added for some context):

GameControllerTest.java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
@Test
public void gameShouldEndOnWin() {
    exit.expectSystemExit();

    controller.newGame();

    // Select two human players
    view.enqueueInput("h");
    view.enqueueInput("h");

    controller.setUp();


    view.enqueueInput("middle center"); // Player 1's first move
    view.enqueueInput("top left");      // Player 2's first move
    view.enqueueInput("top right");     // Player 1 goes for the diagonal...
    view.enqueueInput("middle left");   // Player 2 goes for the column...
    view.enqueueInput("lower right");   // Player 1 chokes!
    view.enqueueInput("lower left");    // Player 2 wins! What an upset! 

    controller.startGame();
}

I’m sure I’ll discover the shortcomings of this approach sooner or later, but for now it’s a pretty good way to test events inside the main game loop.