05 Jul 2019

Lets do a kata - Tennis in f# - part2

The tennis kata in f# with refactorings - getting advantage -> win

In Part 1 we took a simple path without much thought to get a win condition in the game. After writing this code I was vaguely happy. The score types were a bit suspect

 type PlayerAScore = int
 type PlayerBScore = int

but we got to green and got our win done fast, we were not depending on primitive types, we had a tell dont ask pattern and a simple parser to convert state to score. Really all things considered it was ok.

Then we decided to go for a plugable set of sinks to handle the parsing of the state into a score. The code was left with a bit of duplication across a couple of functions and there is a fair bit of function nesting. I am not sure how I feel about this - I don’t really want to break out modules and I do want functions to be scoped appropriately.

Lets figure out how to do Deuce->Advantage->Win

We nailed the early game, lets move onto the part of the game that can go on forever. If you want to just jump to the end and see the code then click here

Test for Deuce

[<Fact>]
let ``Score is deuce when it is 40-40``() =
    let state = initial_state
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)

    Assert.Equal("Deuce", score state)

Nothing unexpected here, is it obvious how to implement? I think so. We just create another score parser and stick it in the right place in the pipeline (above early game scorer).

Deuce implementation

    let advantage_game_scorer state =
        let apply_if_in_advantage (to_apply: GameState -> ScoreState, game_state: GameState): ScoreState =
            match game_state.playerAScore > 2 && game_state.playerBScore > 2 with
            | true -> to_apply (game_state)
            | false -> Unhandled game_state

        let deuce_scorer game_state =
            match game_state.playerAScore = game_state.playerBScore with
            | true -> Scored "Deuce"
            | false -> Unhandled game_state

        let deuce_scorer_handler state = apply_if_in_advantage(deuce_scorer, state)

        state
        |> apply_if_unscored (deuce_scorer_handler)

and then in the pipeline

 (Unhandled state)
    |> early_game_win_scorer
    |> advantage_game_scorer
    |> early_game_scorer
    |> score_state_to_string

At this point I decide I don’t like snake case for function names. I am not entirely sure what made me use it at all for function names. Probably Elixir.

Turns out Deuce was easy

So I am feeling good about this refactor - but I am becoming very aware that there is an increasing amount of duplication.

Lets write a test for advantage

We need to look at deuce->advantage and advantage->deuce

[<Fact>]
let ``Score is advantage player a  when it is player A scores after deuce``() =
    let state = initial_state
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerAScores)

    Assert.Equal("Adv PlayerA", score state)

[<Fact>]
let ``Score is advantage player b  when it is player b scores after deuce``() =
    let state = initial_state
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)

    Assert.Equal("Adv PlayerB", score state)

[<Fact>]
let ``Score is deuce when player A scores after player B advantage``() =
    let state = initial_state
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)

    Assert.Equal("Deuce", score state)

Implementation for advantage

It feels like the refactor is still paying off. Lets approach advantage by simply plugging another parser into the pipeline of the advantageGameScorer

        let advantageAScorer game_state =
            match game_state.playerAScore > game_state.playerBScore with
            | true -> Scored "Adv PlayerA"
            | false -> Unhandled game_state

        let advantageBScorer game_state =
            match game_state.playerBScore > game_state.playerAScore with
            | true -> Scored "Adv PlayerB"
            | false -> Unhandled game_state

        let deuceScorerHandler state = applyIfInAdvantage(deuceScorer, state)
        let advantageAHandler state = applyIfInAdvantage(advantageAScorer, state)
        let advantageBHandler state = applyIfInAdvantage(advantageBScorer, state)

and the hookup into the pipeline of advantageGameScorer

        state
        |> applyIfUnscored deuceScorerHandler
        |> applyIfUnscored advantageAHandler
        |> applyIfUnscored advantageBHandler

So this is going pretty well. We have a v simple pattern in principle - a load of nested sinks and we just flow the data through until it catches on the right gear. Duplication is going up, the functions that return the string are all looking v similar.

Lets write a test for winning the advantage game

[<Fact>]
let ``Player A wins when scoring after advantage``() =
    let state = initial_state
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerAScores)
                |> handle (PlayerAScores)

    Assert.Equal("Player A Won", score state)
    
[<Fact>]
let ``Player B wins when scoring after advantage``() =
    let state = initial_state
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerAScores)
                |> handle (PlayerBScores)
                |> handle (PlayerBScores)

    Assert.Equal("Player B Won", score state)

amazingly just more of the same.

Implementation of winning.

This is going to look very very similar …inside advantageGameScorer

        let winnerPlayerA game_state =
            match game_state.playerAScore > game_state.playerBScore + 1 with
            | true -> Scored "Player A Won"
            | false -> Unhandled game_state
            
        let winnerPlayerB game_state =
            match game_state.playerBScore > game_state.playerAScore + 1 with
            | true -> Scored "Player B Won"
            | false -> Unhandled game_state

then again in advantageGameScorer it is time to hook into the pipeline

        state
        |> applyIfUnscored winnerPlayerAHandler
        |> applyIfUnscored winnerPlayerBHandler
        |> applyIfUnscored deuceScorerHandler
        |> applyIfUnscored advantageAHandler
        |> applyIfUnscored advantageBHandler

I really like this pipeline thing, the nesting means as we are adding behavior to the advantage part everything else stays unmodified. The only thing I have a problem with is that the game logic is part of the output logic.

Lets extract the conditional

    let scoreGame condition result game_state =
        match condition with
        | true -> Scored result
        | false -> Unhandled game_state

now we can replace all those match statements with a simple function call Eg

scoreGame (simpleWinCondition (state.playerAScore, state.playerBScore)) "Player A Won" state
scoreGame (simpleWinCondition (state.playerBScore, state.playerAScore)) "Player B Won" state

we can also tidyup all the calls to applyIfInAdvantage by only entering the pipe if that is true (rather than prefix every advantage pipe with a check)

If we change the function definition of applyIfInAdvantage from

let applyIfInAdvantage (to_apply: GameState -> ScoreState, game_state: GameState): ScoreState =

to

let applyIfInAdvantage (to_apply: GameState -> ScoreState) game_state

then piping and currying will work. IE we can curry on the to_apply function then take the result of that and put that into the pipe.

the pipeline inside advantageGameScorer ends up looking like this

        let apply_scorers (state : GameState) =
            Unhandled state
            |> applyIfUnscored winnerPlayerA
            |> applyIfUnscored winnerPlayerB
            |> applyIfUnscored deuceScorer
            |> applyIfUnscored advantageAScorer
            |> applyIfUnscored advantageBScorer

        state |> applyIfUnscored(applyIfInAdvantage(apply_scorers))

So now we have a far neater solution but … applyIfUnscored feels bad

This was a pattern we chose, but it is everywhere. Its an infestation. There must be a better way. Because all we want is a string and because we will at some point convert state to string - ut we don’t know where we need this check everywhere.

What if we figure out how to make the to string bit simple by already figuring out what the score is? Maybe approach that from the command handler?

We have a problem - Tennis is infinite

The approach taken here works, but we have used an int to model the score. Whilst an int is a larger datatype than any game of tennis ever it does place a finite limit on an otherwise infinite game. This is a smell that points to our problem of a naive command handler and it makes me want to try another direction.

The solution? I have no idea, but we can refactor all this into a different solution

The advantage of TDD is you end up with a set of tests and an API you trust. You can then do all kinds of fun behind the scenes.

Coming in Part 3 : Moving the logic out of the display function and into the command handler

This will be messy and hard. I actually ended up breaking the existing implementation because I slept inbetween the work, came back in the morning and rather than duplicating, modifying and repointing I modified in place. Really Really Bad.

Anyway on to Part 3