You can't sink me

Here is a Battlehips game in F#. Now, before you start jumping up and down on my code, please understand that, for all its awesomeness, F# has a serious limitation: Its compilation is linear, and the solution structure cannot have folders. Any code that uses other code must be below the other code, whether in the same file, or in another file lower than the first file in the compilation sequence.

Here is my GeneralData file. It contains basic data and methods not specifically relevant to other groups.

module GeneralData

type direction = Horizontal | Vertical

type public point = {x :int; y :int}
type public battleship = {coordinate :point; direction :direction; length :int}

let charToDirection (character :char) :direction =
    match character with
    | 'H' | 'h' -> Horizontal
    | 'V' | 'v' -> Vertical
    | _ -> raise  'H'
    | Vertical -> 'V'

Next in the compilation hierarchy is my Board class:

```
module Board

open GeneralData
open System

[]
type public squareState = Empty = 0 | Ship = 1 | Bombarded = 2

type public Board() =
let mutable board :squareState[,] = Array2D.init 10 10 (fun x y -> squareState.Empty)

let getChar (state :squareState) :char =
if state.HasFlag(squareState.Bombarded) && state.HasFlag(squareState.Ship) then
'X'
elif state.HasFlag(squareState.Bombarded) then
'O'
else
' '

let getCharShowingShips (state :squareState) :char =
if state.HasFlag(squareState.Bombarded) && state.HasFlag(squareState.Ship) then
'X'
elif state.HasFlag(squareState.Bombarded) then
'O'
elif state.HasFlag(squareState.Ship) then
'S'
else
' '

member self.boardArray :squareState[,] = board

member public self.placeShip(ship :battleship) =
for i = 0 to ship.length - 1 do
// This member should only be called before bombardment starts,
// which means that all sq

You are correct in your statement that this is a pretty standard OOP implementation. The key signs are lots of mutable state and explicit loops. It's hard to give a line-by-line critique because a functional program would be structured so differently (maybe this is why nobody has posted an answer yet!)

First let's tackle a small stylistic point: It's generally accepted that in F#, types and type members are PascalCase, whereas function names are camelCase.

Another thing that stands out is that the functions in your code are quite long. A common consequence of FP, and in particular designing with types, is that you end up with smaller functions at every level, which can be composed and used at the higher level.

A common temptation when modelling two-dimensional boards such as in Battleships (or Chess, Noughts and Crosses etc) is to try and model the physical nature of the board - ie, a 2-dimensional array. However, when you think about the domain, this is only superficial. What you really have on a "board" is a series of points/coordinates, a set of ships which occupy some of those points, and a series of guesses (again, points). With the 2d-array approach, you're immediately restricted to working with primitive values by indexing into the array. Although you had the right idea by creating the point record type, you didn't leverage the ability to work on the point level - instead, all the functions which use the point type are further up the conceptual hierarchy, mainly in the Board type. The mental model of the array restricts your thinking to X and Y values, rather than dealing in points.

I'd like to give a longer answer but I'm out of time right now, I'll gladly respond to any questions you have! I started working on a functional version of the problem, I only got as far as the "domain" (no input events or rendering yet) but perhaps this will help you visualise how a functional version would be structured:

module Common =
    type Result = 
     | Success of 'TSuccess 
     | Error of 'TError

module Seq =
    let overlaps (xs: 'a seq) (ys: 'a seq) = Seq.exists2 (=) xs ys

module Domain =
    type Direction = Horizontal | Vertical

    type Point = {
        X: int
        Y: int
    } with
        static member zero = { X = 0; Y = 0 }
        static member addX (p: Point) (i: int) = { p with X = p.X + i }
        static member addY (p: Point) (i: int) = { p with Y = p.Y + i }
        static member add = function
            | Horizontal -> Point.addX
            | Vertical -> Point.addY
        static member (>~=) (a: Point, b: Point) =
            a.X >= b.X && a.Y >= b.Y

    type Ship = {
        Name: string
        Length: int
    }

    let ships = [
        { Name = "Aircraft Carrier"; Length = 5 }
        { Name = "Battleship"; Length = 4 }
        { Name = "Frigate"; Length = 3 }
        { Name = "Submarine"; Length = 3 }
        { Name = "Minesweeper"; Length = 2 }
    ]

    type PlacedShip = {
        Ship: Ship
        Position: Point
        Direction: Direction
    } with
        static member allPoints (ps: PlacedShip) =
            List.init ps.Ship.Length (Point.add ps.Direction ps.Position)
        static member lastPoint (ps: PlacedShip) =
            Point.add ps.Direction ps.Position (ps.Ship.Length - 1)

    type BoardState = {
        Ships: PlacedShip list
        Guessed: Set
        Bounds: Point
    } with
        static member allPlacedPoints (bs: BoardState) =
            bs.Ships
            |> Seq.collect PlacedShip.allPoints
            |> Set.ofSeq

        static member placeShip (bs: BoardState) (s: PlacedShip) =
            let withinBounds =
                bs.Bounds >~= (PlacedShip.lastPoint s) && s.Position >~= Point.zero

            let overlapsOtherShips =
                bs
                |> BoardState.allPlacedPoints
                |> Seq.overlaps (PlacedShip.allPoints s)

            if not withinBounds then
                Common.Error "This ship placement is out of bounds"
            else if overlapsOtherShips then
                Common.Error "This ship placement overlaps another ship"
            else
                Common.Success { bs with Ships = s :: bs.Ships }

        static member addGuess (bs: BoardState) (p: Point) =
            if Set.contains p bs.Guessed then
                Common.Error  BoardState.allPlacedPoints
            |> Set.isSubset bs.Guessed

    type Player = {
        Name: string
        Board: BoardState
    }

StackExchange Code Review Q#141166, answer score: 8