First-time F#: A simple Rock Paper Scissors

Question

This is my first F# program. It's a simple Rock-Paper-Scissors implementation.

I am looking for feedback of all sorts, e.g. the choice of types (e.g. List vs Array), whether there are parts that could be solved more elegantly / idiomatic, formatting, etc.

open System

let allowedActions = [ "r"; "p"; "s" ]

let randomItem list (rng: Random) =
    List.item (rng.Next(List.length list)) list

let computerAction () = randomItem allowedActions (Random())
let rec playerAction () =
    printfn "Write 'r' for Rock, 'p' for Paper, 's' for Scissors. Type 'q' to Quit."

    let input = Console.ReadLine()

    let isInputAllowed = List.contains input allowedActions || input = "q"
    match isInputAllowed with
    | true -> input
    | _ -> printfn "invalid input '%s'" input; playerAction()

let computerWin playerAction computerAction =
    printfn "The computer did win. '%s' beats '%s'." computerAction playerAction
let playerWin playerAction computerAction =
    printfn "The player did win. '%s' beats '%s'." playerAction computerAction
let tie anAction = printfn "It's a tie. '%s'" anAction

let ruling playerAction computerAction =
    match (playerAction, computerAction) with
    | ("r", "p")
    | ("p", "s")
    | ("s", "r") -> computerWin playerAction computerAction
    | ("r", "s")
    | ("p", "r")
    | ("s", "p") -> playerWin playerAction computerAction
    | (_, _) -> tie playerAction

let playGame playerAction computerAction =
    printfn "You've chosen '%s'." playerAction
    printfn "The computer chose '%s'." computerAction
    ruling playerAction computerAction
    ()

let game () =
    let playerAction = playerAction ()
    let computerAction = computerAction ()
    match playerAction with
    | "q" -> false
    | _ ->
        playGame playerAction computerAction
        true

[<EntryPoint>]
let rec main arg =
    printfn "Hello World to Rock Paper Scissors"
    match game () with
    | true -> main arg
    | false -> 0
```

Answer 1

As a first attempt it looks in many ways ok.

I have three "complaints":

a) You rely too much on string literals as identifiers for input. That is considered bad practice in - I think - every programming language and should be avoided because it's error prone (typos and illegal input etc.) hence not very robust.

Instead of "r", "p" and "s" you could use a discriminated union - which is a very important part of the F#-toolbox to be familiar with:

type Weapon =
    | Rock
    | Paper
    | Scissors

This will for instance make the match-statement in ruling much more readable and type safe:

let ruling playerWeapon computerWeapon =
    match (playerWeapon, computerWeapon) with
    | (Rock, Paper)
    | (Paper, Scissors)
    | (Scissors, Rock) -> computerWin playerWeapon computerWeapon
    | (Rock, Scissors)
    | (Paper, Rock)
    | (Scissors, Paper) -> playerWin playerWeapon computerWeapon
    | (_, _) -> tie playerWeapon

---

b) Don't repeat yourself. In the functions where you print the results, you do the same thing, and therefore you should extract the common parts to a generalized function, so that:

let computerWin playerAction computerAction =
    printfn "The computer did win. '%s' beats '%s'." computerAction playerAction
let playerWin playerAction computerAction =
    printfn "The player did win. '%s' beats '%s'." playerAction computerAction

changes to:

let showWinner name winnerWeapon looserWeapon = 
    printfn "The %s did win. '%A' beats '%A'." name winnerWeapon looserWeapon
let computerWin playerWeapon computerWeapon = showWinner "computer" computerWeapon playerWeapon
let playerWin playerWeapon computerWeapon = showWinner "player" playerWeapon computerWeapon

---

c) You recursively call the main function. I don't know if you actually violate any formal or informal rules but it just looks "ugly" to me. I would make a dedicated function that runs the game recursively.

---

FYI - I have below refactored your program while incorporating my suggestions above:

// HH: Instead of string literals you should use discriminated unions as identifiers for "weapons"
//     This is much more robust in respect to typos etc.
type Weapon =
    | Rock
    | Paper
    | Scissors
    static member Abbreviation w = (w.ToString().[0]).ToString().ToLower()
    static member ToWeapon ch =
        match ch with
        | "r" -> Rock
        | "p" -> Paper
        | "s" -> Scissors
        | _ -> failwith "Invalid Weapon char"

let weapons = [ Rock; Paper; Scissors ]

// HH: You should only instantiate a single random object - used throughout the session.
let rand = new Random()
let computerAction () = weapons.[rand.Next(weapons.Length)]


// HH: This now returns an optional value of None if the user wants to quit
//     or Some (Weapon) if a valid weapon is chosen
let rec playerAction () =
    let allowedActions = weapons |> List.map Weapon.Abbreviation
    let choices = weapons |> List.map (fun w -> sprintf "'%s' = %A" (Weapon.Abbreviation w) w)
    printfn "Enter:\n%s.\n'q' to Quit." (String.Join("\n", choices))
    let input = Console.ReadLine()

    let validWeapon w = List.contains w allowedActions
    match input with
    | "q" -> None
    | w when validWeapon w -> Some (Weapon.ToWeapon w)
    | _ -> printfn "invalid input '%s'" input; playerAction()

//HH: Never repeat yourself: extract a function to print the winner...
let showWinner name winnerWeapon looserWeapon = 
    printfn "The %s did win. '%A' beats '%A'." name winnerWeapon looserWeapon
// HH: ... and call that from the dedicated winner functions
let computerWin playerWeapon computerWeapon = showWinner "computer" computerWeapon playerWeapon
let playerWin playerWeapon computerWeapon = showWinner "player" playerWeapon computerWeapon
let tie anAction = printfn "It's a tie. '%A'" anAction

let ruling playerWeapon computerWeapon =
    // HH: By using discriminated unions this match
    //     expression is much more readble and robust
    match (playerWeapon, computerWeapon) with
    | (Rock, Paper)
    | (Paper, Scissors)
    | (Scissors, Rock) -> computerWin playerWeapon computerWeapon
    | (Rock, Scissors)
    | (Paper, Rock)
    | (Scissors, Paper) -> playerWin playerWeapon computerWeapon
    | (_, _) -> tie playerWeapon

let playGame playerWeapon computerWeapon =
    printfn "You've chosen '%A'." playerWeapon
    printfn "The computer chose '%A'." computerWeapon
    ruling playerWeapon computerWeapon
    ()

let runGame () =
    let playerAction = playerAction ()
    match playerAction with
    | Some playerWeapon ->
        let computerWeapon = computerAction ()
        playGame playerWeapon computerWeapon
        true
    | None -> false

// HH: Personally I don't like,that you call main recursively.
//     You probably don't violate any formal or informal rule, but it just look wrong to me
//     So make a dedicated function to start the game by
let rec play () =
    match runGame() with
    | true -> play()
    | false -> ()

I have changed some of the names so that they matches my naming. For the example I show that you can extent a discriminated union with members (static and instance). You could chose to have these methods as normal function instead.