HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Checkers moves using union types

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
movesunionusingtypescheckers

Problem

How do I refactor common code with different union types?

I'm still struggling with refactoring F# code with different types but same behavior.

I have the following function that I would like to refactor:

let movePiece destination positions piece =

    let foundPiece = positions |> List.filter (fun space -> space = Allocated piece)
                               |> List.head
    let destinationXY = 
        match destination with
        | Available xy -> xy
        | Allocated p  -> coordinateOf p

    let canCrown =
        let yValue = snd destinationXY
        (yValue = 0 || yValue = 7) && 
        not (isKing piece)

    match foundPiece with
    | Allocated (Black (ch, xy)) -> let checkerType = match canCrown with
                                                      | true  -> BlackKing
                                                      | false -> BlackSoldier

                                    (positions |> List.filter (fun space -> space <> Allocated (Black (ch, xy)))
                                               |> List.filter (fun space -> space <> destination))
                                               @  [Available (xy) ; (Allocated (Black (checkerType, destinationXY)))]

    | Allocated (Red   (ch, xy)) -> let checkerType = match canCrown with
                                                      | true  -> RedKing
                                                      | false -> RedSoldier

                                    (positions |> List.filter (fun space -> space <> Allocated (Red (ch, xy)))
                                               |> List.filter (fun space -> space <> destination))
                                               @  [Available (xy) ; (Allocated (Red   (checkerType, destinationXY)))]
    | _ -> positions


Here's the entire domain:

```
( Types )
type Black = BlackKing | BlackSoldier
type Red = RedKing | RedSoldier

type Coordinate = int * int

type Piece =
| Black of Black * Coordinate
|

Solution

The biggest flaw in your model is that you don't model Color as a type. If you do that, you can eliminate a lot of duplication in other pieces and can make most code color agnostic.

The second big change I'd make is not coupling the coordinate so tightly with pieces or spaces. I'd only add this information in the places that require it.

type Color = Black | Red

type PieceKind = Soldier | King

type Piece = Color * PieceKind

type SquareContent = // If you want, you can use `Option` here.
    | Occupied of Piece
    | Empty

type Coordinate = int * int

type Result = // If you want, you can inline `Result` into `Status`, but I wouldn't recommend it.
    | Win of Color
    | Draw

type Status =
    | Turn of Color
    | Ended of Result


I'd represent the board as a mapping from Coordinate to SquareContent, not as a list of Coordinate * SquareContent tuples.

If you don't use the above redesign, you should rename your current Black and Red, because they don't represent those colors, they represent a piece with that color. So call them BlackPiece and RedPiece.

Code Snippets

type Color = Black | Red

type PieceKind = Soldier | King

type Piece = Color * PieceKind

type SquareContent = // If you want, you can use `Option<Piece>` here.
    | Occupied of Piece
    | Empty

type Coordinate = int * int

type Result = // If you want, you can inline `Result` into `Status`, but I wouldn't recommend it.
    | Win of Color
    | Draw

type Status =
    | Turn of Color
    | Ended of Result

Context

StackExchange Code Review Q#137932, answer score: 4

Revisions (0)

No revisions yet.