3

I have a standard deck of 52 cards is represented in an array. Each card is represented as an integer. I wrote the below function to shuffle the cards. Does the code below look ok?

Module Module3

Sub Main()

    ' initialize array
    Dim Cards(52) As Integer

    ' Unit Test
    ' Pass array as argument.
    Console.WriteLine(shuffle(Cards))

End Sub

Function shuffle(ByVal Cards() As Integer)

    Dim counter = 1
    Dim rand = New Random()

    For Each card In Cards

        ' Grab random number with range of 52
        Dim n = rand.Next(52)

        ' Pick a card
        Dim temp = Cards(counter)

        ' Swap picked card with random card
        Cards(counter) = Cards(n)
        Cards(n) = temp

        counter += 1

    Next

    Return (Cards)

End Function

End Module
Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
Bruno
  • 6,211
  • 16
  • 69
  • 104
  • possible duplicate of [Is there an easy way to randomize a list in VB.NET?](http://stackoverflow.com/questions/554587/is-there-an-easy-way-to-randomize-a-list-in-vb-net) – Hans Passant Nov 13 '11 at 00:39
  • @Bruno - consider using a list. See the code I provided. – dbasnett Nov 13 '11 at 15:28

4 Answers4

4

No, the code doesn't do what you say.

Dim Cards(52) As Integer

This will create an array for 53 cards, not 52. Use:

Dim Cards(51) As Integer

When shuffling, swap each card with a card earlier in the deck (or itself), not anywhere in the deck. (This is the principle of the Fisher-Yates shuffle.)

Instead of having a counter separate from the loop, use the counter for the looping.

Dim rand = New Random()

For counter = 0 to Cards.Length - 1

  Dim n = rand.Next(counter + 1)

  Dim temp = Cards(counter)
  Cards(counter) = Cards(n)
  Cards(n) = temp

Next
Guffa
  • 687,336
  • 108
  • 737
  • 1,005
1

Take a look at Jeff Atwood's blog post on the subject.

http://www.codinghorror.com/blog/2007/12/shuffling.html

Rob Windsor
  • 6,744
  • 1
  • 21
  • 26
1

If this is going to be used for some game I wouldn't use an array, I would use a list because of the ease items can be added / removed.

Module Module1

    Dim deck As New List(Of Integer)
    Dim prng As New Random

    Sub Main()
        newDeck()
        showDeck()
        shuffle()
        showDeck()
        Dim s As String
        Do
            s = Console.ReadLine()
            Select Case s
                Case "s"
                    showDeck()
                Case "c"
                    If deck.Count = 0 Then
                        Console.WriteLine("No cards")
                    Else
                        'take top card
                        Dim foo As Integer = deck(0)
                        deck.RemoveAt(0)
                        Console.WriteLine(foo.ToString)
                    End If
                Case "n"
                    newDeck()
                    shuffle()
            End Select
        Loop Until s.ToLower = "x"
    End Sub

    Sub newDeck()
        deck = Enumerable.Range(1, 52).ToList 'create deck
    End Sub

    Sub shuffle()
        deck = deck.OrderBy(Function(r) prng.Next).ToList
    End Sub

    Sub showDeck()
        Dim ctr As Integer = 0
        Console.WriteLine()
        For Each card As Integer In deck
            Console.Write("{0}", card.ToString.PadLeft(4, " "c))
            ctr += 1
            If ctr Mod 10 = 0 Then Console.WriteLine()
        Next
        Console.WriteLine()
    End Sub
End Module

For some time I have thought that this was a case where the computer shouldn't mimic the real world exactly. First, here is a program that shows the problem with knowing the seed of the random number generator.

'create a form with three buttons and a richtextbox
Dim deckIdx As New List(Of Integer)
Dim cards As New List(Of card)
Dim prng As New Random

Private Sub Button1_Click(sender As System.Object, e As System.EventArgs) Handles Button1.Click
    newDeck()
    shuffle()
    showDeck()
End Sub

Private Sub Button3_Click(sender As System.Object, e As System.EventArgs) Handles Button3.Click
    'show next five cards
    Dim sb As New System.Text.StringBuilder
    sb.AppendLine()
    For x As Integer = 1 To 5
        Dim foo As card = getTopCard()
        If Not foo Is Nothing Then sb.AppendFormat("{0}", foo.theCard.ToString.PadLeft(4, " "c))
    Next
    RichTextBox1.AppendText(sb.ToString)
    RichTextBox1.ScrollToCaret()
End Sub

Private Sub Form1_Shown(sender As Object, e As System.EventArgs) Handles Me.Shown
    Button1.PerformClick()
End Sub

Class card
    Public theCard As Integer
    Public count As Integer
End Class

Sub newDeck()
    prng = New Random(42) '<<<<<<<<<<<<<<<<<<<<<<<< OOPS!!!
    deckIdx = Enumerable.Range(0, 51).ToList 'create deck indicies
    cards = New List(Of card)
    For Each cIDX As Integer In deckIdx
        Dim foo As New card
        foo.theCard = cIDX
        foo.count = 0
        cards.Add(foo)
    Next
End Sub

Sub shuffle()
    deckIdx = deckIdx.OrderBy(Function(r) prng.Next).ToList
End Sub

Function getTopCard() As card
    If deckIdx.Count > 0 Then
        Dim foo As New card
        foo.theCard = cards(deckIdx(0)).theCard
        foo.count = cards(deckIdx(0)).count
        deckIdx.RemoveAt(0)
        Return foo
    Else
        Return Nothing
    End If
End Function

Sub showDeck()
    Dim ctr As Integer = 0
    Dim sb As New System.Text.StringBuilder
    For Each card As Integer In deckIdx
        sb.AppendFormat("{0}", cards(card).theCard.ToString.PadLeft(4, " "c))
        ctr += 1
        If ctr Mod 10 = 0 Then sb.AppendLine()
    Next
    RichTextBox1.Text = sb.ToString
End Sub

Running the program and pressing button1 / 3 repeatedly will produce the same results, over and over. If we are going to adhere to the idea that the computer must mimic the real world exactly, then the obvious fix is to not re-seed the PRNG. But what if we took a different approach.

Add this code

Dim bkgShuffle As Threading.Thread
Private Sub Button2_Click(sender As System.Object, e As System.EventArgs) Handles Button2.Click
    Button2.Enabled = False
    bkgShuffle = New Threading.Thread(AddressOf shuffleBkg)
    bkgShuffle.IsBackground = True
    bkgShuffle.Start()
End Sub

Sub shuffleBkg()
    Do
        Threading.Thread.Sleep(50)
        shuffle()
    Loop
End Sub

Run the program as before, press button 1 and 3 to verify that nothing has changed. When you are satisfied press button 2, and then press button 1 and 3.

When you play cards in person it is obvious that the deck is not constantly being shuffled, but what if it was? Would it change anything? If I play cards online do I know or care if the deck is constantly being shuffled?

The code was only to demonstrate a point, to think outside of the box, and should not be considered a finished product.

edit: Bull Mountain is likely to affect some of this.

dbasnett
  • 11,334
  • 2
  • 25
  • 33
0

This is formally now known as the 'Briggs' Shuffle

Module module1

Dim cards(51) As String
Dim trues(51) As Boolean
Dim number, Truecheck As Integer
Dim stores, loopy As String

Sub main()

    number = 1

    cards(0) = "Ace of Spades"
    cards(10) = "Jack of Spades"
    cards(11) = "Queen of Spades"
    cards(12) = "King of Spades"

    cards(13) = "Ace of Clubs"
    cards(23) = "Jack of Clubs"
    cards(24) = "Queen of Clubs"
    cards(25) = "King of Clubs"

    cards(26) = "Aec of Diamonds"
    cards(36) = "Jack of Diamods"
    cards(37) = "Queen of Diamonds"
    cards(38) = "King of Diamonds"

    cards(39) = "Ace of Hearts"
    cards(49) = "Jack of Heats"
    cards(50) = "Queen of Hearts"
    cards(51) = "King of Hearts"


    For i = 1 To 9
        number = number + 1
        cards(i) = number.ToString + " of Spades"
    Next

    number = 1

    For i = 14 To 22
        number = number + 1
        cards(i) = number.ToString + " of Clubs"
    Next

    number = 1

    For i = 27 To 35
        number = number + 1
        cards(i) = number.ToString + " of Diamonds"
    Next

    number = 1

    For i = 40 To 48
        number = number + 1
        cards(i) = number.ToString + " of Hearts"
    Next

    For i = 0 To 51
        Console.WriteLine(cards(i))
    Next


    Console.WriteLine("")
    Console.WriteLine("")

    For i = 0 To 51

linetrue:

        Randomize()
        stores = cards(i)
        Truecheck = Int(Rnd() * 51)
        If trues(Truecheck) = True Then GoTo linetrue

        trues(i) = True
        cards(i) = cards(Truecheck)
        cards(Truecheck) = stores
        Console.WriteLine(cards(i))

    Next


End Sub


End Module
Briggs
  • 1