"Rock, Paper, Scissors +" in F#

Question

During a discussion in a development chat, a user suggested to another (in the context of C# originally),

UserA: Challenge: Create an implementation of RPS, and then show how it can be extended to include an unlockable option Bomb (blows up paper and rock, but scissors cuts the fuse - basically Paper+; AI that has Bomb uses it instead of Paper) with a minimum of fuss.

UserA: Bonus: Make it so the game can be thus modified changing only data (constants, serialized fields, whatever).

UserB: how would one unlock the Bomb?

UserA: Basically "elsewhere" however you want. Could be an external property, could be an Interface or Serialized Field or parameter or whatever.

and since I'm interested in learning F# I decided to pick up the exercise. Along with that, I also tried to clean up the code for I/O handling, such that I could potentially swap a "player vs computer" game to "player vs player" or even "computer vs computer". I have published the script in GitHub, but please find it pasted below too for convenience:

open System

type MatchResult =
    | Won
    | Lost
    | Draw

type Element =
    | Rock
    | Paper
    | Scissors
    | Bomb

type Player = {
    Pick: Element[] -> Element;
    AnnounceResult: MatchResult -> unit;
}

let victoriesOf element =
    match element with
    | Rock -> [Scissors]
    | Paper -> [Rock]
    | Scissors -> [Paper; Bomb]
    | Bomb -> [Rock; Paper]

let (!) (result: MatchResult) =
    match result with
    | Won -> Lost
    | Lost -> Won
    | x -> x

let evaluate player rival =
    let winsOver a b =
        victoriesOf a
        |> List.contains b
    match (winsOver player rival, winsOver rival player) with
    | (true, false) -> Won
    | (false, true) -> Lost
    | _ -> Draw

let parse options move =
    let cleanup (input: string) = input.ToLowerInvariant().Trim()
    let compare =
        string
        >> cleanup
        >> (=) move
    Seq.tryFind compare options

let runRound (a: Player) (b: Player) options =
    let aChoice = a.Pick options
    let bChoice = b.Pick options
    let result = evaluate aChoice bChoice
    a.AnnounceResult result
    b.AnnounceResult !result

let player = {
    Pick = fun options ->
        printfn "Choose one of %s:" (String.Join(", ", options))
        match parse options (Console.ReadLine()) with
        | Some(x) -> x
        | None -> failwith "Unknown choice!";
    AnnounceResult = fun result ->
        match result with
        | Won -> printfn "You won!"
        | Lost -> printfn "You lost!"
        | Draw -> printfn "It's a draw!"
}

let computer = 
    let rng = Random()
    {
        Pick = fun options -> 
            let choice = Array.item (rng.Next(options.Length)) options
            printfn "Computer chose %A" choice
            choice;
        AnnounceResult = fun _ -> ();
    }

[<EntryPoint>]
let main argv =
    let validMoves = [|Rock; Paper; Scissors; Bomb|]

    while true do
        runRound player computer validMoves
    0

Since I'm new to ML-like languages, I'm looking for feedback on how I could better use the F# libraries, better model my state and data, and even how to make it more point-free. And if there are more idiomatic ways of writing any piece of code, please tell!

Answer 1

F# is pretty pragmatic and multi-paradigm - you don't need to stick super hard to ML style code.

type Player = {
    Pick: Element[] -> Element;
    AnnounceResult: MatchResult -> unit;
}

Record fields should be lowercase (and you don't need the semicolons) - but in this case, guidelines suggest using interfaces over a record of functions. Interfaces have their own gotchas in F#, but on the whole are mostly better for this purpose, and also interop.

type Player =
    abstract Pick : Element[] -> Element
    abstract AnnounceResult : MatchResult -> unit

which makes your player (using object expressions and renaming options -> moveOptions):

let player =
    { new Player with
        member _.Pick moveOptions = 
            printfn "Choose one of %s:" (String.Join(", ", moveOptions))
            match parse moveOptions (Console.ReadLine()) with
            | Some x -> x
            | None -> failwith "Unknown choice!"
            
        member _.AnnounceResult result =
            match result with
            | Won -> printfn "You won!"
            | Lost -> printfn "You lost!"
            | Draw -> printfn "It's a draw!"
    }

---

let (!) (result: MatchResult) =
    match result with
    | Won -> Lost
    | Lost -> Won
    | x -> x

This would shadow the ref cell dereference operator - even though you don't use that here you may want to hide this somewhere closer to where you actually use it; if your module gets opened it might break ref cells in consuming code. You could also stick it on the DU as a member:

type MatchResult =
    | Won
    | Lost
    | Draw
    member x.Not =
        match x with
        | Won -> Lost
        | Lost -> Won
        | Draw -> Draw

and then:

let runRound (a: Player) (b: Player) moveOptions =
    ...
    a.AnnounceResult result
    b.AnnounceResult result.Not

---

Changing some names around here helped me understand this bit better:

let parse moveOptions moveInput =
    let cleanup (input: string) = input.ToLowerInvariant().Trim()
    let isInput =
        string
        >> cleanup
        >> (=) moveInput
    Seq.tryFind isInput moveOptions

Which revealed you're calling string on your Element[] move options list on every player pick. You could just do this once (also, unless we really need Arrays for interop or perf, Lists are simpler to write and work with):

let cleanup (input: string) = input.ToLowerInvariant().Trim()

let main argv =
    let validMoveOptions =
        [ Rock; Paper; Scissors; Bomb ]
        |> List.map (string >> cleanup)
    ...

Or convert the input instead of the list of Elements and skip the compare stuff altogether:

let parseInput =
    let asElement =
        function
        | "rock" -> Rock
        | "paper" -> Paper
        | "scissors" -> Scissors
        | "bomb" -> Bomb
        | s -> failwithf "Unknown move! %s" s
    
    cleanup >> asElement

player becomes:

let player =
    { new Player with
        member _.Pick moveOptions =
            printfn "Choose one of %s:" (String.Join(", ", moveOptions))
            Console.ReadLine() |> parseInput
        member _.AnnounceResult result =
    ... same ...
    }

---

Both victoriesOf and evaluate are hard for me to understand, but I think you could rewrite it like this:

let (|>|) a b = // "beats" operator
    match a, b with
    | Rock, Scissors
    | Paper, Rock
    | Scissors, Paper
    | Scissors, Bomb
    | Bomb, Paper
    | Bomb, Rock -> true
    | _ -> false
    
let rockBeatsPaper = Rock |>| Paper

and evaluate becomes:

let evaluate player rival =
    match player |>| rival, rival |>| player with
    | true, false -> Won
    | false, true -> Lost
    | _ -> Draw

or as a member:

type Element =
    | Rock
    | Paper
    | Scissors
    | Bomb
    member x.Beats y =
        match x, y with
        ...
        
let evaluate player rival =
    match player.Beats rival, rival.Beats player with
    ...

---

If you want to go for that bonus goal, you could drop the Element DU altogether and just work with provided strings via say, cli:

> dotnet run -- -el rock -beats scissors -el paper -beats rock -el scissors -beats paper -beats bomb -el bomb -beats paper -beats rock

Most of the logic doesn't change, you just need to parse the input arguments and parseInput becomes just cleanup.

---

To summarize: convert your input to the model type rather than vice versa. The model types look good. The names chosen got pretty confusing, using 3 different names (move, Element, option) for the same thing made this code harder to understand at first read.