Ggthjy

I am a C# developer learning F#. Would appreciate any suggestions on how this code can be improved. Main points I am interested in:

• Functional style of the code, absense of imperative style(C#) artifacts


• F# collection data types and functions usage


• General code(not algorithm) efficiency


• 
  

  Code style, naming etc.

  
  
  

  let life board =
  
      let flatten = Seq.cast<int> >> Seq.toArray
  
  
  
      let neighboursCount (board: int[,]) x y =
  
          board.[y..y+2, x..x+2]
  
          |> flatten
  
          |> Array.mapi (fun i el -> (i <> 4, el))
  
          |> Array.filter fst |> Array.map snd
  
          |> Array.filter ((=) 1) |> Array.length
  
  
  
      let expandBoard board =
  
          let N = Array2D.length1 board
  
          array2D [|
  
              yield                        [| yield board.[N - 1, N - 1]; yield! board.[N - 1, *]; yield board.[N - 1, 0] |]
  
              for i in [0..N - 1] do yield [| yield board.[i,     N - 1]; yield! board.[i,     *]; yield board.[i,     0] |]
  
              yield                        [| yield board.[0,     N - 1]; yield! board.[0,     *]; yield board.[N - 1, 0] |]
  
          |]
  
  
  
      let getNextBoard board =
  
          let eBoard = expandBoard board
  
          board |> Array2D.mapi (fun i j elem ->
  
              match neighboursCount eBoard j i with
  
              | 2 -> elem
  
              | 3 -> 1
  
              | _ -> 0)
  
  
  
      let isGameOver board prevBoards =
  
          board |> flatten |> Array.forall ((=) 0) || List.contains board prevBoards
  
  
  
      let rec lifeRec board prevBoards =
  
          printfn "%A n" board
  
          if isGameOver board prevBoards
  
          then ()
  
          else getNextBoard board |> lifeRec <| board::prevBoards
  
  
  
      lifeRec board []
  
  
  
  let N = 5
  
  let rnd = System.Random()
  
  Array2D.init N N (fun _ _ -> rnd.Next 2) |> life
  
  

  
  

This is a most elegant solution. I have two small improvements:

let neighboursCount (board: int[,]) x y =

    board.[y..y+2, x..x+2] 

    |> flatten 

    |> Array.indexed

    |> Array.sumBy (fun (i, n) -> if i <> 4 then n else 0)

let expandBoard board =

    let N = (Array2D.length1 board) - 1

    array2D [|

        yield                    [| yield board.[N, N]; yield! board.[N, *]; yield board.[N, 0] |]

        for i in [0..N] do yield [| yield board.[i, N]; yield! board.[i, *]; yield board.[i, 0] |]

        yield                    [| yield board.[0, N]; yield! board.[0, *]; yield board.[N, 0] |]

    |]

Here is N set to (Array2D.length1 board) - 1 because that's the value you're using through out the function.

Review for the updated version (because there are no answers yet, I think you can still remove the first version).

I really like your implementation. Especially the idea of expanding the board instead of calculating the opposide indices as i did). That makes the code much more readable / understandable. I also like the way you imlemented the neighboursCount function - the combination of mapi, filter and map is a good example for functional programming.

In fact, I don't see much room for improvement.

Maybe you could rename neighboursCount into countNeighbours an move the board to the end of the argument list so it can be used like: eBorad |> countNeighbours j i

Within the isGameOver function, I think I would prefer writing prevBoards |> List.contains board over List.contains board prevBoards.

Thanks for sharing your solution :).