3
\$\begingroup\$

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 the deck, it converts the number to the actual value and symbols of cards, doing some math to convert it in such a way that number 1 becomes Ace of Diamonds and number 52 becomes King of Clubs. Versive sub also allows multiple cards drawn at once.

\$\endgroup\$
2
\$\begingroup\$

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

\$\endgroup\$
  • \$\begingroup\$ I actually had the idea to make the naming "funny" when I saw an example code that was "marine", so the first line read "Sub marine" I will make the naming better. \$\endgroup\$ – Moacir Nov 30 '16 at 13:31
1
\$\begingroup\$

Overall looks pretty efficient to me. I built a euchre game many years ago in dos using basic, I believe. One sub to build an array of numbers and symbols as you have done here. Of course VBA makes it much simpler. Yours is a fun approach. I ran your code flawlessly in 2010. I can't imagine how to make it faster. I would add to assylias' comment to be very descriptive in your module, variable, array, and subroutine names. It helps other programmers down the road, but it will help yourself too if you revisit it ten years from now....

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.