3
\$\begingroup\$

I am working on a simple Texas Hold'Em dealing simulator. I am new to Python and was hoping to get some advice on my use of classes here. I feel like I am not using them as efficiently as possible, especially when dealing out the player cards. Specifically, I feel like the def deal(self, n_players) could be simplified instead of using playercard1 and playercard2, and deleting the items from the "deck".

from random import shuffle

class TexasHoldem:
    #create deck
    def __init__(self):
        values = ['Ace', '2', '3', '4', '5', '6', '7', '8', '9', '10', 'Jack', 'Queen', 'King']
        suites = ['Heart', 'Spade', 'Club', 'Diamond']
        self.deck = [j + " " + i for j in values for i in suites]

    #shuffle deck
    def shuffle(self):
        shuffle(self.deck)

    #deal for players
    def deal(self, n_players):
        count = 0
        #card 1 list (will be shuffled, delt 'around the table' as if a real stack)
        playercard1 = list()
        while count < n_players:
            card1 = self.deck[count]
            playercard1.append(card1)
            count += 1

        #remove cards from deck that were delt 
        for i in playercard1:
            self.deck.remove(i)

         count = 0
         #card 2 list 
         playercard2 = list() 
         while count < n_players:
             card2 = self.deck[count]
             playercard2.append(card2)
             count += 1

         #remove cards from deck delt for 2nd card
         for i in playercard2:
             self.deck.remove(i)

        #merge cards of playercard1 and playercard2 into set     
        self.playerhand = zip(playercard1, playercard2)

    #define the flop    
    def flop(self):
        #burn a card
        del self.deck[0]
        #lay down three
        self.flopcards = self.deck[0:3]
        #delete flop from deck
        for i in self.flopcards:
            self.deck.remove(i)

    #same as flop for turn and river        
    def turn(self):
        del self.deck[0]
        self.turncard = self.deck[0:1]

        for i in self.turncard:   
            self.deck.remove(i)

    def river(self):
        del self.deck[0]
        self.rivercard = self.deck[0:1]

        for i in self.rivercard:
            self.deck.remove(i)

#create instance of Cards class            
c = TexasHoldem()
#shuffle for this instance
c.shuffle()

#deal for n players
c.deal(9)

print 'Player Hands:'
for i in c.playerhand:
    print i

#flop
c.flop()
print 'Flop:'
print c.flopcards

#turn
c.turn()
print 'Turn:'
print c.turncard

#river
print 'River:'
c.river()
print c.rivercard
\$\endgroup\$
2
\$\begingroup\$

Your code does a few things repetitively:

count = 0
playercard1 = list()
while count < n_players:
    card1 = self.deck[count]
    playercard1.append(card1)
    count += 1

for i in playercard1:
    self.deck.remove(i)

I'll go through the problems with this:

  • You manually increment count, this is something that you should do with range.
  • You manually index every item.
  • You manually append to a list.
  • You manually remove each card from the deck, this causes the list to shift, and causes a small search of the list. (\$O(n)\$ complexity)

Instead you could either:

  1. Pop the amount of cards you need.

    pop = self.deck.pop
    playercard1 = [pop() for _ in range(n_players)]
    
  2. Slice the amount of items you need and then delete them.

    playercard1 = self.deck[-n_players:]
    del self.deck[-n_players:]
    

You should notice that I take from the end of the list, this is as you aren't using a queue and so they will be faster.


The other main code you use is:

del self.deck[0]
self.flopcards = self.deck[0:3]
del self.deck[0:3]

This uses the same code as before, but burns one card. You can then just make two functions to do what most of your code is doing at the moment.

def take(self, number_cards):
    pop = self.deck.pop
    return [pop() for _ in range(number_cards)]

def draw(self, number_cards):
    self.deck.pop()
    return self.take(number_cards)

This then makes the rest of your code simple and easy to read and use.

def deal(self, n_players):
    self.playerhand = zip(self.take(n_players), self.take(n_players))

def flop(self):
    self.flopcards = self.draw(3)

def turn(self):
    self.turncard = self.draw(1)

def river(self):
    self.rivercard = self.draw(1)

Other than the above I'd:

  • Add shuffle to your __init__ so that the cards are pre-shuffled.
  • Move the deck creation into shuffle to allow more than one game, and
  • Move values and suites to the global scope as 'constants'.

This results in something like:

VALUES = ['Ace', '2', '3', '4', '5', '6', '7', '8', '9', '10', 'Jack', 'Queen', 'King']
SUITES = ['Heart', 'Spade', 'Club', 'Diamond']
class TexasHoldem:
    def __init__(self):
        self.shuffle()

    def shuffle(self):
        self.deck = [j + " " + i for j in VALUES for i in SUITES]
        shuffle(self.deck)
\$\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.