16 July 2013

Word Wrap Kata revisited

Last year in November, my old post about Word Wrap Kata variants suddenly got a lot of visits. Claus told me that well known German blogger Ralf Westphal had linked to it from one of his own articles about Test Driven Development. By using examples from Coding Dojos he explained that using Test Driven Development alone is not enough to get clean code. During the Dojo the participants focused only on the red and green bars and other concerns like design or code readability were neglected. He used my code as examples of solutions got by such a behaviour. While it made me unhappy to serve as negative example, I had to agree with Ralf. When I performed the katas two years ago I explored different algorithms and did not use them to practice TDD. My solutions were technical, instead of reflecting the problem domain.

Try Harder in Your New DreamWhen I attended a Coding Dojo afterwards I paid close attention and was sad to see he that all Ralf's concerns came true. The group did not even understand the requirements completely, when people already started proposing language features and libraries which would solve the problem quickly. Solve which problem quickly? We should think about the problem first and then create code driven by our tests that is clean, readable and open for future change. Ralf had some ideas how to improve these issues behind programming. Obviously code needs to have many qualities beyond its simple correctness.

So I tried again. I got help from my friend Thomas Sundberg and we worked five remote pairing sessions on the Word Wrap code kata. We performed the kata in the "usual" way, without much thought up front and being driven by our tests. Similar to a Coderetreat, we chose additional constraints: We focused on business related names, SRP and Tell, don't ask. As a kata is a learning exercise, we took everything to the extreme. We literally spent hours discussing if a particular name was reflecting the problem domain, renaming it three times or more until we were satisfied. And we spent at least six pomodoros entirely on refactoring and cleaning up. Probably we could still improve it but we grew tired of the exercise and switched to another kata.

When I see the code now, it looks a bit weird, likely because taking "Tell, don't ask" to the extreme means never asking for any state. So the first thing we need is a class that receives the wrapped output, as we cannot ask for it. Word Wrap is an algorithm inside a word processor when a paragraph of text, which does not contain any newlines, is rendered to a page where it needs to be broken into lines of proper length:
interface Page {
    void renderLine(String lineOfProperLength);
}
In breaking the paragraph into lines of proper length, we see several responsibilities: splitting the paragraph,
interface ParagraphSplitter {
    void wrap(String paragraph);
}
which breaks the stream of text down into words, accumulating the words into lines of proper length,
interface LineAccumulator {
    void addAndHyphenateIfNeeded(String word);
    void addWithoutHyphenation(String part);
    void addCarriageReturn();
}
and maybe a hyphenation rule to determine if a word which is too long can be broken into shorter pieces,
interface HyphenationRule {
    void doHyphenate(String word, LineAccumulator lineAccumulator);
}
We did not start with this design but arrived at it when removing duplication and multiple responsibilities mercilessly ;-) The obvious implementation of ParagraphSplitter is to split on each blank, e.g.
class SeparateWordsOnBlanks implements ParagraphSplitter {

    private final LineAccumulator accumulator;

    // constructor omitted

    public void wrap(String paragraph) {
        for (String word : paragraph.split(BLANK)) {
            accumulator.addAndHyphenateIfNeeded(word);
        }
        accumulator.addCarriageReturn();
    }
}
which is not exciting at all. The heavy lifting is done by the accumulator which checks if adding the current word would exceed the maximum line length, invokes the hyphenation and adds blanks where needed. Whenever a line is complete it is rendered to the page.
class LineLengthAccumulator implements LineAccumulator {

    private final Page page;
    private final HyphenationRule hyphenation;
    private final int maximumLineLength;
    private StringBuilder currentLine = new StringBuilder();

    // constructor omitted

    public void addAndHyphenateIfNeeded(String word) {
        if (exceedingMaximumLineLength(SPACE, word)) {
            hyphenation.doHyphenate(word, this);
            return;
        }
        insertSpaceIfNeeded();
        appendToCurrentLine(word);
    }

    public void addWithoutHyphenation(String part) {
        if (exceedingMaximumLineLength(EMPTY, part)) {
            addCarriageReturn();
        }
        appendToCurrentLine(part);
    }

    private boolean exceedingMaximumLineLength(String separator, String word) {
        return currentLine.length() + separator.length() + word.length() > maximumLineLength;
    }

    public void addCarriageReturn() {
        if (currentLineHasWords()) {
            renderCurrentLine();
            lineFeed();
        }
    }

    // some private methods omitted

}
There are a bunch of unit tests using a NoneHyphenationRule or different anonymous mock rules to drive the functionality of the LineLengthAccumulator. For example
@Test
public void shouldNotWrap() {
    Page mockOutput = mock(Page.class);

    LineAccumulator accumulator = new LineLengthAccumulator(mockOutput, 78);
    accumulator.addAndHyphenateIfNeeded("This");
    accumulator.addAndHyphenateIfNeeded("is");
    accumulator.addCarriageReturn();

    verify(mockOutput).renderLine("This is");
    verify(mockOutput, times(1)).renderLine(anyString());
}
The HyphenationRule needs to know its accumulator and the other way round, which creates a conceptual cycle. Additionally we need a second method addWithoutHyphenation in the LineAccumulator to avoid endless loops during hyphenation. This is the result of following "Tell, don't ask" to the letter. Maybe HyphenationRule would be better off just returning the hyphenated word.
class SplitOnCamelCase implements HyphenationRule {

    public void doHyphenate(String word, LineAccumulator lineAccumulator) {
        if (foundAnUpperCaseLetterIn(word)) {
            String remainingWords = splitFirstSyllableFrom(word, lineAccumulator);
            doHyphenate(remainingWords, lineAccumulator);
        } else {
            lineAccumulator.addWithoutHyphenation(word);
        }
    }

    // private methods omitted
}
To unit test the hyphenation rules, a mock accumulator is used to verify the proper syllables.
@Test
public void shouldHyphenateWords() {
    LineAccumulator mockAccumulator = mock(LineAccumulator.class);
    HyphenationRule strategy = new SplitOnCamelCase();

    strategy.doHyphenate("ShortWord", mockAccumulator);

    InOrder inOrder = inOrder(mockAccumulator);
    inOrder.verify(mockAccumulator).addWithoutHyphenation("Short");
    inOrder.verify(mockAccumulator).addWithoutHyphenation("Word");
    verify(mockAccumulator, times(2)).addWithoutHyphenation(anyString());
}
I am pleased with our result, although it took us too long to complete it, which is a sign how much practice we still need ;-) See its full source code here. You might still find one or another thing that is not optimal and never will be but in comparison with my first version this Word Wrap expresses its problem domain more clearly and is easy to understand even without digging down into all the details.

No comments: