.net - Shuffling an array of strings in vb.net

Question

I am developing a webpage in vb.net that will generate a number of multiple choice questions to the user. I need to shuffle the four answers which are already put in an array. Lets assume that I have to following array:

array = {"Correct", "Wrong1", "Wrong2", "Wrong3"}

I tried to use the following method:

Public Shared Function Shuffle(ByVal items() As String) As Array
        Dim max_index As Integer = items.Length - 1
        Dim rnd As New Random(DateTime.Now.Millisecond)
        For i As Integer = 0 To max_index
            ' Pick an item for position i.
            Randomize()
            Dim j As Integer = rnd.Next(i, max_index)
            ' Swap them.
            Dim temp As String = items(i)
            items(i) = items(j)
            items(j) = temp
        Next i
        Return items
    End Function

The function is working really fine, but my problem is that if I have four questions, the answers will be shuffled for each question but the correct answer will be in one position like:

    array = {"Wrong1", "Correct", "Wrong2", "Wrong3"}
    array = {"Wrong2", "Correct", "Wrong3", "Wrong1"}
    array = {"Wrong3", "Correct", "Wrong1", "Wrong2"}
    array = {"Wrong1", "Correct", "Wrong3", "Wrong2"}

What I need is the correct answer to change its location from one question to another. Your help is appreciated.

Answer 1

There are several issues with your your Shuffle method and use of Random.

1. Randomize is for use with the old, legacy VB Rnd function. It has no impact on the shiny new NET Random class.
2. There is rarely a need to provide a custom seed; in fact this can work against you.

3. Create one Random for the entire app to use rather than each time you shuffle (or click something). And never create them in a loop - this almost guarantees repeated numbers.

4. The max argument to Random.Next(min, max) is exclusive, so your range is actually 1 element smaller than it should be.

5. Your Shuffle is pretty close, but has a few flaws:
   • Generally the NET version of the Fisher-Yates Shuffle is an in place shuffle (Sub) making it very efficient (rather than returning a new collection)
   • It is important to loops backwards thru the list or array.¹

The Standard Fisher-Yates Shuffle:

' form/class level var
Private rnd As New Random()

Public Sub Shuffle(items As String())
    Dim j As Int32
    Dim temp As String

    For n As Int32 = items.Length - 1 To 0 Step -1
        j = rnd.Next(0, n + 1)
        ' Swap them.
        temp = items(n)
        items(n) = items(j)
        items(j) = temp
    Next n
End Sub

Output from 4 shuffles of the same starting array:

Shuffle #1    Wrong3   Correct  Wrong2   Wrong1   
Shuffle #2    Correct  Wrong2   Wrong1    Wrong3   
Shuffle #3    Wrong2   Correct  Wrong3    Wrong1   
Shuffle #4    Correct  Wrong1   Wrong2    Wrong3   
Shuffle #5    Correct  Wrong1   Wrong3    Wrong2   

Variations

Rather than a global Random generator, you can pass it (useful as an extension method):

Public Sub Shuffle(items As String(), RNG As Random)

Generic method for shuffling many types:

' Generic shuffle for basic type arrays
Public Sub Shuffle(Of T)(items As T(), rng As Random)
    Dim temp As T
    Dim j As Int32

    For i As Int32 = items.Count - 1 To 0 Step -1
        ' Pick an item for position i.
        j = rng.Next(i + 1)
        ' Swap 
        temp = items(i)
        items(i) = items(j)
        items(j) = temp
    Next i
End Sub

Examples:

Shuffle(intArray, myRnd)
Shuffle(strArray, myRnd)
Shuffle(colors, myRnd)
Shuffle(myButtons, myRnd)

Simple Reordering

Finally, for something simple where you might only reorder them once, a simple and usually-good-enough version using an extension method:

Dim ShuffledItems = myItems.OrderBy(Function() rnd.Next).ToArray()

This has less code and is easy to dash off, but is much less efficient.

¹ The reason that you want to loop backwards is to limit each array element to one swap. Once "X" is moved to items(j) in the last step, that position is removed from consideration because rnd.Next(0, n + 1) will only pick up to the last element/loop index. Many implementations get this wrong.