Deck Shuffling Method

Question

Just wondering if you guys think this is a good way to shuffle a deck or a bad way. Updated to show the GetRandom function as requested.

Private Sub DeckShuffle()
        ' shuffles the deck 4 times.
        For shufflingCards As Integer = 0 To 3
            ' goes through each card and changes position once each time it goes through shufflingCards
            For shuffleEachCard As Integer = 0 To 51
                ' gets a random number 0 - 51
                Dim placeHolder As Integer = GetRandom(0, 51)
                ' creates a temporary card for swapping cards
                Dim tempCard As Card = deck(placeHolder)
                ' makes the random number card now the card we were shuffling
                deck(placeHolder) = deck(shuffleEachCard)
                ' replaces the card we were shuffling with the one we were saving as a temporary
                deck(shuffleEachCard) = tempCard
            Next
        Next
    End Sub

' Gets a random number for shuffling.
Private Function GetRandom(ByVal Min As Integer, ByVal Max As Integer) As Integer
    Static Generator As System.Random = New System.Random()
    Return Generator.Next(Min, Max)
End Function

Answer 1

You won't need to shuffle the deck 4 times. For a use case of shuffling a deck of cards using the System.Random once should be sufficient.

But there is still something to say about the GetRandom() method:

• based on the .NET naming Guidelines method arguments should be named using camelCase casing. Although VB.NET is case-insensitive you should improve your coding style by applying the guidelines because other developers will expect it.
• the declaration and initialisation of Static Generator As System.Random = New System.Random() could be simplified like Static generator As New System.Random() which takes into account naming local variables using camelCase casing as well.

---

DeckShuffle()

• If this method is part of a Deck class you should just name it Shuffle otherwise ShuffleDeck would be better because methods should be named using verbs or verb phrases.

• The inner loop variable shuffleEachCard isn't named good as well. currentCard would be more significant (you can't shuffle a single card).

• Dim placeHolder As Integer = GetRandom(0, 51) will get you a random number in the range of 0..50 because the maxValue parameter of Random.Next is exclusive. Hence you should change it to GetRandom(0, 52) to get a random number in the range 0..51. In addition randomCardNumber would be a better name.

Answer 2

Another way to shuffle a deck is to start of with your full deck (oldDeck) and an empty deck (newDeck). This uses the same random generator function in the OP. I haven't tried to properly code this because the algorithm is the important message.

While size of oldDeck > 0
  chosenCard = getrandom(0,size of oldDeck-1)
  remove chosenCard from oldDeck ' reduces the size of oldDeck by 1
  append chosenCard to newDeck
'Wend
return newDeck ' or reassign newDeck to oldDeck, depending on the broader context

This algorithm would be particularly useful if a class of Deck existed, and it could be a member function as noted in the previous reply. You could then consider having a class of Card which includes a suit and value. A Deck would then be a collection of Cards, and the built-in collection functions become your friends. Of course, this amount of effort depends on what you are going to do with your Deck.

Answer 3

@Brandon, you only need one pass through the deck.

A question: why don't you try the Fisher-Yates shuffle?

Pseudocode from Wikipedia article:

-- To shuffle an array a of n elements (indices 0..n-1):
for i from 0 to n−2 do
     j ← random integer such that i ≤ j < n
     exchange a[i] and a[j]

Your code would look like this:

        For shuffleEachCard As Integer = 0 To 50
            ' gets a random number 
            Dim placeHolder As Integer = GetRandom(i, 52)
            ' creates a temporary card for swapping cards
            Dim tempCard As Card = deck(placeHolder)
            ' makes the random number card now the card we were shuffling
            deck(placeHolder) = deck(shuffleEachCard)
            ' replaces the card we were shuffling with the one we were saving as a temporary
            deck(shuffleEachCard) = tempCard
        Next