Skip to main content
Spelling and grammar
Source Link
Toby Speight
  • 81.8k
  • 14
  • 101
  • 309

I had some smaller points to make on several aspects of your code but I won't mention them since the whole thing can be written in more idiomatic and simplesimpler style without using any recursion or mutation.

Note the use of Seq.sumBy and Array.countBy to cut down on a lot of the work:

let random = System.Random()

let heads () = random.Next(0, 2) = 1

let calculateHeads n =
    { 1 .. n } |> Seq.sumBy (fun _ -> if heads () then 1 else 0)

let getFrequencies n m =
    let counts =
        Array.init m (fun _ -> calculateHeads n)
        |> Array.countBy id
        |> Map
    Array.init (n + 1) (fun i -> counts.TryFind i |> Option.defaultValue 0)

let displayCoin frequency =
    for c in frequency do
        printfn "%d%s -- (%d)" c (String.replicate (c / 10) "*") c

let frequency = getFrequencies 32 1000
displayCoin frequency

I also made random a module level value instead of passing it around everywhere. The downside of this is you can't test individual functions deterministically.

If you're not already doing so, you should send code to F# interactive in your IDE to get a much faster feedback cycle for trying out ideas and exploring data.

I had some smaller points to make on several aspects of your code but I won't mention them since the whole thing can be written more idiomatic and simple style without using any recursion or mutation.

Note the use of Seq.sumBy and Array.countBy to cut down on a lot of the work:

let random = System.Random()

let heads () = random.Next(0, 2) = 1

let calculateHeads n =
    { 1 .. n } |> Seq.sumBy (fun _ -> if heads () then 1 else 0)

let getFrequencies n m =
    let counts =
        Array.init m (fun _ -> calculateHeads n)
        |> Array.countBy id
        |> Map
    Array.init (n + 1) (fun i -> counts.TryFind i |> Option.defaultValue 0)

let displayCoin frequency =
    for c in frequency do
        printfn "%d%s -- (%d)" c (String.replicate (c / 10) "*") c

let frequency = getFrequencies 32 1000
displayCoin frequency

I also made random a module level value instead of passing it around everywhere. The downside of this is you can't test individual functions deterministically.

If you're not already doing so, you should send code to F# interactive in your IDE to get a much faster feedback cycle for trying out ideas and exploring data.

I had some smaller points to make on several aspects of your code but I won't mention them since the whole thing can be written in more idiomatic and simpler style without using any recursion or mutation.

Note the use of Seq.sumBy and Array.countBy to cut down on a lot of the work:

let random = System.Random()

let heads () = random.Next(0, 2) = 1

let calculateHeads n =
    { 1 .. n } |> Seq.sumBy (fun _ -> if heads () then 1 else 0)

let getFrequencies n m =
    let counts =
        Array.init m (fun _ -> calculateHeads n)
        |> Array.countBy id
        |> Map
    Array.init (n + 1) (fun i -> counts.TryFind i |> Option.defaultValue 0)

let displayCoin frequency =
    for c in frequency do
        printfn "%d%s -- (%d)" c (String.replicate (c / 10) "*") c

let frequency = getFrequencies 32 1000
displayCoin frequency

I also made random a module level value instead of passing it around everywhere. The downside of this is you can't test individual functions deterministically.

If you're not already doing so, you should send code to F# interactive in your IDE to get a much faster feedback cycle for trying out ideas and exploring data.

Source Link

I had some smaller points to make on several aspects of your code but I won't mention them since the whole thing can be written more idiomatic and simple style without using any recursion or mutation.

Note the use of Seq.sumBy and Array.countBy to cut down on a lot of the work:

let random = System.Random()

let heads () = random.Next(0, 2) = 1

let calculateHeads n =
    { 1 .. n } |> Seq.sumBy (fun _ -> if heads () then 1 else 0)

let getFrequencies n m =
    let counts =
        Array.init m (fun _ -> calculateHeads n)
        |> Array.countBy id
        |> Map
    Array.init (n + 1) (fun i -> counts.TryFind i |> Option.defaultValue 0)

let displayCoin frequency =
    for c in frequency do
        printfn "%d%s -- (%d)" c (String.replicate (c / 10) "*") c

let frequency = getFrequencies 32 1000
displayCoin frequency

I also made random a module level value instead of passing it around everywhere. The downside of this is you can't test individual functions deterministically.

If you're not already doing so, you should send code to F# interactive in your IDE to get a much faster feedback cycle for trying out ideas and exploring data.