8
\$\begingroup\$

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
    
\$\endgroup\$
1
\$\begingroup\$

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.

\$\endgroup\$
0
\$\begingroup\$

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 :).

\$\endgroup\$
  • \$\begingroup\$ I believe neighboursCount came from my OO background, where you name object properties like that. Also that is a bit unusual that I can use piping where it seems to be unneded, but in fact version with piping is much more readable, so I accept your point. \$\endgroup\$ – Andrew Boklashko Dec 13 '18 at 10:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.