HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Deck shuffle and drawing cards

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
cardsdeckdrawingshuffleand

Problem

I developed a simple deck shuffle and drawing system, as a test on how to use collections and learn from it.

However, I think my code is a bit wonky, and I think here is the best place to get new tips on what to improve.

Public deck As New Collection
Public face As New Collection
Public symbols As New Collection

Sub atomic()

face.Add "A"
For i = 2 To 10
    face.Add i
Next i
face.Add "J"
face.Add "Q"
face.Add "K"
symbols.Add ChrW(9826)
symbols.Add ChrW(9828)
symbols.Add ChrW(9825)
symbols.Add ChrW(9831)

End Sub

Sub lime()

Set deck = Nothing
Set face = Nothing
Set symbols = Nothing
Dim i As Long

For i = 1 To 52
deck.Add i
Next i

atomic

End Sub

Sub versive(draw As Long)
Dim randc As Long
Dim ub As Long
Dim cardnumber As Long
Dim actualcard As String
Dim actualcard2 As String
Dim i As Long
Dim lastrow As Long
lastrow = Cells(Rows.count, 14).End(xlUp).row
For i = 1 To draw

    ub = deck.count
    If ub = 0 Then
        MsgBox "Out of cards"
    Exit Sub
    End If
    randc = Int((ub - 1 + 1) * Rnd() + 1)
    If randc > ub Then
       randc = ub
    End If
    cardnumber = deck.Item(randc)
    deck.Remove (randc)
    actualcard = face.Item(cardnumber - (WorksheetFunction.RoundUp(cardnumber / 13, 0) - 1) * 13) & symbols.Item(WorksheetFunction.RoundUp(cardnumber / 13, 0))
    lastrow = lastrow + 1
    Cells(lastrow, 14).Value = actualcard
Next i
End Sub

Private Sub CommandButton1_Click()
versive (51)
End Sub

Private Sub CommandButton2_Click()
lime
End Sub


If it matters, it is developed on Excel VBA.

The atomic sub calls for the definition of the cards names, because it was not just numbers (Like 1 is actually "A") and the definition for the symbols (Hearts, diamonds, etc).

The lime sub basically restarts everything. It runs the atomic sub, so lime actually needs to run first. It also defines that the deck will have 52 cards.

The versive sub actually draws cards. So after it gets a random number from 1 to the amount of cards left in th

Solution

Your naming is funny and all but quite counter-productive - names primary goal is to explain what the code does, not to give a lol moment (I know, boring).


The atomic sub calls for the definition of the cards names

then call it DefineCards


The lime sub basically restarts everything

then call it Restart


The versive sub actually draws cards

then call it DrawCards

Context

StackExchange Code Review Q#148546, answer score: 2

Revisions (0)

No revisions yet.