don't worry, it's probably fine

15 Minute Rock-Paper-Scissors with Test Commit Revert

09 Nov 2019

nablopomo tdd java

It’s been a while since I did a code exercise.

I decided to revisit Rock-Paper-Scissors which we’d tackled during a pairing session at work last month.

I also wanted to take my test-commit-revert gradle plugin for a spin again.

Test-Commit-Revert is a micro-practice that compels you to make your changes as small (and thus low-risk) as possible - the journey to a functionally complete piece of code is paved with lots of very small, safe commits.

If your tests pass, that’s an automatic commit. If they fail, then the change is rolled back.

Here’s how it went.

Rock

Rather than try to implement the whole thing at one, I opted to be guided by “0, 1, Many”.

The simplest version of the game is just “Rock” - each player can only throw “Rock”, and the outcome can only be a draw.

@Test
public void first() {
    assertThat(play("ROCK", "ROCK"), is("DRAW"));
}

private Object play(String rock, String rock1) {
    return "DRAW";
}

I didn’t bother with types because play was generated by my IDE.

Rock-Paper

The next goal was to add another move - now the players were engaging in the not-quite-as-good game “Rock-Paper” and there winners and losers.

@Test
public void second() {
    assertThat(play("PAPER","ROCK"), is("P1"));
    assertThat(play("ROCK","PAPER"), is("P2"));
}

private Object play(String move1, String move2) {
    if("PAPER".equals(move1) && "ROCK".equals(move2)) {
        return "P1";
    } else if ("ROCK".equals(move1) && "PAPER".equals(move2)) {
        return "P2";
    } else {
        return "DRAW";
    }
}

This worked fine but the code was starting to get ugly. Time to refactor.

Refactoring

The type of move and the results of the game were both well-defined sets of “things”. A player could only (at this point) throw Rock or Paper, and the outcome was either a Player 1 win, Player 2 win, or a draw.

This lends itself naturally to using an enum to replace the String values that were being passed around.

After the course of many safe commits, and using the “strangler” pattern to provide a new entry point, the code looked a fair bit nicer.

Plus, there were now stronger types to guide the IDE (and myself).

@Test
public void second() {
    assertThat(play(PAPER, ROCK), is(P1));
    assertThat(play(ROCK, PAPER), is(P2));
}

private Outcome play(Throw move1, Throw move2) {
    if(move1 == PAPER && move2 == ROCK) {
        return P1;
    } else if (move1 == ROCK && move2 == PAPER) {
        return P2;
    } else {
        return DRAW;
    }
}

This if-else was about to get even hairier with the addition of “Scissors” so I decided to do a bit of pre-factoring.

Welcome to the Matrix

Rather than an if-else, Rock-Paper can be modelled as a lookup in a 2x2 array, where the first index was the move of Player 1, and the second index the move of Player 2.

I wanted to replace the logic-based code with something like results[player1Move][player2Move].

Since we’re using Test-Commit-Revert, I felt happy hacking around a bit, safe in the knowledge that code that broke the tests would get thrown away.

I ended up with this bit of code.

private Outcome play(Throw move1, Throw move2) {
    return new Outcome[][] {
        { DRAW, P2 },
        { P1, DRAW }
    }[move1.ordinal()][move2.ordinal()];
}

Shorter, but not necessarily more readable. Hmm. Well, I thought, let’s carry on and see where it leads.

Rock-Paper-Scissors

In theory adding support for the third move, Scissors, would only require expanding the grid from 2x2 to 3x3.

Growing a grid from NxN to (N+1)x(N+1) requires adding 2N + 1 entries and their corresponding 2N + 1 tests, in this case 5.

Each commit was adding more test code than implementation code.

I ended up with the following, after a bit of refactoring:

private static final Outcome[][] ROCK_PAPER_SCISSORS = {
    {DRAW, P2, P1},
    {P1, DRAW, P2},
    {P2, P1, DRAW}
};

private Outcome play(Throw move1, Throw move2) {
    return ROCK_PAPER_SCISSORS[move1.ordinal()][move2.ordinal()];
}

The sharp-eyed will notice that the grid is inversely symmetrical down the diagonal made of DRAW - if there’s a P1 below the diagonal, its “reflection” will be a P2. This reflects (hahaha) that if the players swap moves, the winner swaps too.

I like this for terseness, but I’m in two minds about readability - if I’d never seen this code before, would I understand immediately that the each row in the grid corresponded to a different move by player 1?

I think this approach has legs if there was a cleaner way of building this grid than an array constructer of array constructors.

Conclusion

All in all, it took me about 15m from start to finish, not including time spent faffing around with my test-commit-revert plugin before realising it wasn’t handling my PGP auto-sign very well.

One thing I like about Test-Commit-Revert is that I find myself deliberately structuring my code such that small changes are easy, in a way that I sometimes don’t do if I’m trying to build something for real.

I’m aiming to store all of these sorts of thing in my public exercises repository on GitHub.

The problem with Test-Commit-Revert is that the commit history looks like garbage - another feature for the plugin, perhaps, would be to prompt for a one-line commit message.


November is National Blog Posting Month, or NaBloPoMo. I’ll be endeavouring to write one blog post per day in the month of November 2019 - some short and sweet, others long and boring.