Can this random number generator logic be simplified?

Question

I am learning F# and would like to know if the following logic to generate random numbers is acceptable.

Could this be written in a more maintainable fashion? Is this code safe?

let hashset = System.Collections.Generic.HashSet<int>()
let mutable continueLooping = true

while (continueLooping) do 
   let value = System.Random().Next(0, 12)
   let success = hashset.Add(value)
   continueLooping <- hashset.Count <> 12

UPDATE

let hashset = System.Collections.Generic.HashSet<int>()
let randomGenerator = System.Random()
let mutable continueLooping = true
let expectedLength = 12

while (continueLooping) do 
   let value = randomGenerator.Next(0, expectedLength)
   let success = hashset.Add(value)
   continueLooping <- hashset.Count <> expectedLength

Isn't this just an inefficient way of creating a set containing `[0..11]`? — Lee, Nov 22 '15 at 19:29
I wanted unique numbers. Again, I'm just asking for guidance... — Scott Nimrod, Nov 22 '15 at 19:30
Possible duplicate of [How can I select a random value from a list using F#](http://stackoverflow.com/questions/33312260/how-can-i-select-a-random-value-from-a-list-using-f) — Mark Seemann, Nov 22 '15 at 19:30
oops yes - I saw to late - it's indeed a bit strange that you pull 12 random numbers out of `0..11` inside a hashset that will loose any ordering of those — Random Dev, Nov 22 '15 at 19:32
As outlined [here](http://stackoverflow.com/a/33316100/126014), create any list, and then shuffle it afterwards. — Mark Seemann, Nov 22 '15 at 19:35

score 4 · Accepted Answer · answered Nov 22 '15 at 22:57

You could define a helper function to perform a Fisher-Yates shuffle. This shuffle function is pretty generally useful since it will work on any seq<'a> so you have plenty of opportunities to reuse it.

// shuffle a sequence into random order
let shuffle xs =
    // swap two elements in the supplied array
    let swap i j (array : _[]) =
        let tmp = array.[i]
        array.[i] <- array.[j]
        array.[j] <- tmp
    let rnd = System.Random()
    let xArray = Seq.toArray xs
    let n = Array.length xArray
    for i in [0..(n-2)] do
        let j = rnd.Next(i, n-1)
        swap i j xArray
    xArray |> Seq.ofArray

Then just apply it to a list or something using

let shuffledList = [0..11] |> shuffle |> Seq.toList

Can this random number generator logic be simplified?

1 Answers1

Linked