7
\$\begingroup\$

I'm an absolute python beginner.

I created a draw poker game that works fine up to about ten thousand hands, at which point it starts to freeze and not generate the hands. Given that straight flushes only generate about once per 70 thousand hands (it may be more common bc my program uses multiple decks for the many hands) and royal flushes once per >600k, I'd like to be able to generate more hands. Is there something in my code that's taking up tons of memory or slowing the process down?

Here's my current code:

import copy
import distutils.core
from random import randint
from math import floor

#Individual Cards
class Card:
    def __init__ (self,value,suit):
        self.value = value
        self.suit = suit
        self.vname = ''
        self.sname = ''

    def valname(self, value):
        if self.value == 2:
            self.vname = 'Two'
            return 'Two'
        if self.value == 3:
            self.vname = 'Three'
            return 'Three'
        if self.value == 4:
            self.vname = 'Four'
            return'Four'
        if self.value == 5:
            self.vname = 'Five'
            return'Five'
        if self.value == 6:
            self.vname = 'Six'
            return'Six'
        if self.value == 7:
            self.vname = 'Seven'
            return'Seven'
        if self.value == 8:
            self.vname = 'Eight'
            return'Eight'
        if self.value == 9:
            self.vname = 'Nine'
            return'Nine'
        if self.value == 10:
            self.vname = 'Ten'
            return'Ten'
        if self.value == 11:
            self.vname = 'Jack'
            return'Jack'
        if self.value == 12:
            self.vname = 'Queen'
            return'Queen'
        if self.value == 13:
            self.vname = 'King'
            return'King'
        if self.value == 14:
            self.vname = 'Ace'
            return'Ace'

    def suitname(self, suit):
        if self.suit == "hearts":
            self.sname = '♥'
            return '♥'
        if self.suit == "spades":
            self.sname = '♠'
            return '♠'
        if self.suit == "clubs":
            self.sname = '♣'
            return '♣'
        if self.suit == "diamonds":
            self.sname = '♦︎'
            return '♦︎'

    def cardname(self):
        return f'{self.sname}{self.vname}{self.sname}'

#All Decks
class Deck:
    def __init__(self):
        self.cards = []
        self.create()

    def create(self):
        for d in range(decks):
            for suit in ["hearts", "spades", "clubs", "diamonds"]:
                for val in [2,3,4,5,6,7,8,9,10,11,12,13,14]:
                    self.cards.append(Card(val,suit)); d+=1

    def shuffle(self):
        for _ in range(26):
            for index in range(len(self.cards)-1,-1,-1):
                rindex = randint(0, index)
                self.cards[index], self.cards[rindex] = self.cards[rindex], self.cards[index]

    def draw(self):
        c1 = self.cards.pop()
        c2 = self.cards.pop()
        c3 = self.cards.pop()
        c4 = self.cards.pop()
        c5 = self.cards.pop()
        return [c1,c2,c3,c4,c5]

def ss():
    if show_strength: print(f'[{round(strength/10000,6)}]')

#Evaluation Functions
def evalname(x):
    if x == 2:
        return 'Two'
    if x == 3:
        return 'Three'
    if x == 4:
        return 'Four'
    if x == 5:
        return 'Five'
    if x == 6:
        return 'Six'
    if x == 7:
        return 'Seven'
    if x == 8:
        return 'Eight'
    if x == 9:
        return 'Nine'
    if x == 10:
        return 'Ten'
    if x == 11:
        return 'Jack'
    if x == 12:
        return 'Queen'
    if x == 13:
        return 'King'
    if x == 14:
        return 'Ace'

def hcard(hand):
    global strength
    strength = 1000 + 10*vsort[0] + vsort[1] + .1*vsort[2] + .01*vsort[3] + .001*vsort[4]
    return f'High-Card {evalname(vsort[0])}'

def numpair(hand):
    global strength
    pairs = list(dict.fromkeys([val for val in values if values.count(val) == 2]))
    if len(pairs) < 1:
        return False
    if len(pairs) == 1:
        vp = vsort.copy()
        for _ in range(2):
            vp.remove(pairs[0])
        strength = 2000 + 10*pairs[0] + vp[0] + .1*vp[1] + .01*vp[2];
        return f'Pair of {evalname(pairs[0])}s'
    if len(pairs) == 2:
        vps = vsort.copy()
        for _ in range(2):
            vps.remove(pairs[0]); vps.remove(pairs[1])
        if pairs[0]>pairs[1]:
            strength = (3000 + 10*int(pairs[0]) + int(pairs[1])) + .1*vps[0]
            return f'{evalname(pairs[0])}s and {evalname(pairs[1])}s'
        else:
            strength = (3000 + 10*int(pairs[1]) + int(pairs[0])) + .1*vps[0]
            return f'{evalname(pairs[1])}s and {evalname(pairs[0])}s'


def detset(hand):
    global strength
    detsets = [val for val in values if values.count(val) == 3]
    if len(detsets) < 1:
        return False
    else:
        vs = vsort.copy()
        for _ in range(3):
            vs.remove(detsets[0])
        strength = 4000 + 10*detsets[0] + vs[0] + .1*vs[1]
        return f'Set of {evalname(detsets[0])}s'

def straight(hand):
    global strength
    if (max(vset) - min(vset) == 4) and numpair(hand) == False and detset(hand) == False and quads(hand) == False:
        strength = 4999 + min(vset)
        straight = f'Straight from {evalname(min(vset))} to {evalname(max(vset))}'
    elif vset == {14,2,3,4,5}:
        strength = 5000
        straight = 'Straight from Ace to Five'
    else:
        straight = False
    return straight

def flush(hand):
    global strength
    suits = [hand[0].suit,hand[1].suit,hand[2].suit,hand[3].suit,hand[4].suit]
    flushes = [suit for suit in suits if suits.count(suit) == 5]
    if len(flushes) < 5:
        flush = False
    else:
        values.sort(reverse=True)
        strength = 6000 + 10*values[0] + values[1] + .1*values[2] + .01*values[3] + .001*values[4]
        flush = f'{evalname(max(values))}-High flush of {flushes[0]}'
    return flush

def fullhouse(hand):
    global strength
    pairs = [val for val in values if values.count(val) == 2]
    detsets = [val for val in values if values.count(val) == 3]
    if detset(hand) != False and numpair(hand) != False:
        strength = 7000 + 10*detsets[0] + pairs[0]
        fh = f'{evalname(detsets[0])}s full of {evalname(pairs[0])}s'
    else:
        fh = False
    return fh

def quads(hand):
    global strength
    quads = [val for val in values if values.count(val) == 4]
    if len(quads) < 1:
        return False
    else:
        vq = vsort.copy()
        for _ in range(4):
            vq.remove(quads[0])
        strength = 8000 + 10*quads[0] + vq[0]
        return f'Quad {evalname(quads[0])}s'

def straightflush(hand):
    global strength
    if (max(vset) - min(vset) == 4) and numpair(hand) == False and detset(hand) == False and quads(hand) == False and vset != {14,13,12,11,10}:
        straight = "True"
    elif vset == {14,2,3,4,5}:
        straight = 'Wheel'
    elif vset == {14,13,12,11,10}:
        straight = "Royal"
    else:
        straight = 'False'

    flushes = [suit for suit in suits if suits.count(suit) == 5]
    if len(flushes) < 1:
        flush = False
    else:
        flush = True

    if straight == "True" and flush == True:
        strength = 8999 + min(vset)
        sf = f'{evalname(max(values))}-high straight flush of {flushes[0]}'
    elif straight == "Wheel" and flush == True:
        strength = 9000
        sf = f'Five-High straight flush of {flushes[0]}'
    elif straight == "Royal" and flush == True:
        strength = 10000
        sf = f'Royal flush of {flushes[0]}'
    else:
        sf = False
    return sf

#Count Hand Occurence
hand_occurence = {0:0,1:0,2:0,3:0,4:0,5:0,6:0,7:0,8:0,9:0}
ho_names = ['High Card: ','Pair: ','Two-Pair: ','Three of a Kind: ','Straight: ','Flush: ','Full House: ','Four of a Kind: ','Straight Flush: ','Royal Flush: ']

decks = int(input("How many decks are there? "))
deck = Deck(); deck.shuffle()

hnumber = int(input(f"How many players are there (max {floor((decks*52)/5)})? "))
show_strength = distutils.util.strtobool(input("Would you like to show advanced stats? "))
h_inc = 0; h_strength = {}

while h_inc < hnumber:
    print(f"\nPlayer {h_inc + 1}'s hand:")
    c1,c2,c3,c4,c5 = deck.draw(); hand = c1,c2,c3,c4,c5
    values = [hand[0].value,hand[1].value,hand[2].value,hand[3].value,hand[4].value]; vset = {hand[0].value,hand[1].value,hand[2].value,hand[3].value,hand[4].value}; vsort = sorted(values,reverse=True)
    suits = [hand[0].suit,hand[1].suit,hand[2].suit,hand[3].suit,hand[4].suit]
    c1.valname(c1.value); c2.valname(c2.value); c3.valname(c3.value); c4.valname(c4.value); c5.valname(c5.value);
    c1.suitname(c1.suit); c2.suitname(c2.suit); c3.suitname(c3.suit); c4.suitname(c4.suit); c5.suitname(c5.suit);
    print(f'| {c1.cardname()} | {c2.cardname()} | {c3.cardname()} | {c4.cardname()} | {c5.cardname()} |')

    hcard(hand); numpair(hand); detset(hand); straight(hand); flush(hand); fullhouse(hand); quads(hand); straightflush(hand)
    if strength < 2000:
        print(hcard(hand),end=" "); ss()
        hand_occurence[0]+=1
    elif strength < 3000:
        print(numpair(hand),end=" "); ss()
        hand_occurence[1]+=1
    elif strength < 4000:
        print(numpair(hand),end=" "); ss()
        hand_occurence[2]+=1
    elif strength < 5000:
        print(detset(hand),end=" "); ss()
        hand_occurence[3]+=1
    elif strength < 6000:
        print(straight(hand),end=" "); ss()
        hand_occurence[4]+=1
    elif strength < 7000:
        print(flush(hand),end=" "); ss()
        hand_occurence[5]+=1
    elif strength < 8000:
        print(fullhouse(hand),end=" "); ss()
        hand_occurence[6]+=1
    elif strength < 9000:
        print(quads(hand),end=" "); ss()
        hand_occurence[7]+=1
    elif strength < 10000:
        print(straightflush(hand),end=" "); ss()
        hand_occurence[8]+=1
    elif strength == 10000:
        print(straightflush(hand),end=" "); ss()
        hand_occurence[9]+=1

    h_strength[h_inc] = strength

    h_inc += 1

hss = sorted(h_strength.items(), key=lambda k: k[1], reverse=True)
print(f'\n\n\nPlayer {hss[0][0]+1} has the strongest hand! [{round(hss[0][1]/10000,6)}]\nPlayer {hss[hnumber-1][0] + 1} has the weakest hand :( [{round(hss[hnumber-1][1]/10000,6)}]') if show_strength else print(f'\nPlayer {hss[0][0] + 1} has the strongest hand!\nPlayer {hss[hnumber-1][0]+1} has the weakest hand :(')
if show_strength:

    print('\n\n\n\n\nHand Occurence:\n')
    for x in range(10):
        print(ho_names[x],hand_occurence[x])

    print('\n\n\n\n\nFull Player Ranking:\n')
    for x in range(len(hss)):
        print(f'{x+1}.',f'Player {hss[x][0]+1}',f'[{round(hss[x][1]/10000,6)}]')
\$\endgroup\$
  • \$\begingroup\$ Not sure if I used the word 'draw poker' correctly. This is just a 5-card showdown using holdem hand ranking rules \$\endgroup\$ – Alec Alameddine Feb 25 at 7:25
  • \$\begingroup\$ Welcome to code review. Absolute beginners are welcome here. Enjoy! Hope you learn a lot. \$\endgroup\$ – bhathiya-perera Feb 25 at 11:34
7
\$\begingroup\$

First, some style-issues. Python has an official style-guide, PEP8. It recommends not putting multiple commands on the same line. In addition trailing ; are superfluous.

Now, let's look at the needed structure. You need a Card that contains all information about that card and a Hand which can evaluate a list of cards with respect to the poker rules. You don't actually need a Deck if you just have a list of all cards in the deck and then do random.sample(cards, n_players*5) to get the hands for all players, which you can then distribute to the players.

So, let's have a look at Card first, since you already do have this class. Your valname method is very inefficient. First, it could be that it is called multiple times (this does not seem to be the case). But you also have a chain of ifs, however only ever one of them can be true, so use elif instead. Otherwise all conditions will need to be checked, instead of only all conditions until the first true one.

But even easier here is to use a simple tuple:

class Card:
    value_names = ("Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine", "Ten",
                   "Jack", "Queen", "King", "Ace")
    def __init__(self, value, suit):
        self.value = value
        self.suit = suit

    def __str__(self):
        return f"{self.value_names[self.value - 1]} of {self.suit}"

    def __repr__(self):
        return f"{self.suit} {self.value}"

    def __lt__(self, other):
        return self.value < other.value

    def __eq__(self, other):
        return self.value == other.value

The __str__ method is a magic method that will be called when you do print(card) and __repr__ will be called when typing card in an interactive session. The __lt__ and __eq__ allow cards to be compared by their value, which is used for example by sorted when we have an iterable of cards.

If you want the fancy unicode names for the suits, just use that when constructing the cards:

from itertools import product

deck = [Card(value, suit) for value, suit in product(range(2, 15), "♥♠♣♦")]

Now let's get to the meat of the problem, evaluating poker hands, which should be the responsibility of the Hands class:

from collections import Counter
from itertools import tee, chain


def difference(iterable):
    a, b = tee(iterable)
    try:
        item = next(b)
    except StopIteration:
        return iter([])
    return chain([item], map(lambda x: x[1] - x[0], zip(a, b)))


class Hand:
    def __init__(self, cards):
        self.cards = sorted(cards)
        self.values = [card.value for card in self.cards]
        self.values_counter = Counter(self.values)
        self.suits = [card.suit for card in self.cards]
        self.low_card, self.high_card = self.values[0], self.values[-1]

    def __repr__(self):
        return ", ".join(repr(card) for card in self.cards)

    def flush(self):
        return len(set(self.suits)) == 1

    def straight(self):
        diffs = sorted(list(difference(self.values))[1:])
        return diffs in ([1, 1, 1, 1], [-12, 1, 1, 1])

    def fullhouse(self):
        candidate = self.values_counter.most_common(2)
        return candidate[0][1] == 3 and candidate[1][1] == 2

    def evaluate(self):
        # flush/straight flush/royal flush
        if self.flush():
            if self.straight():
                # royal flush
                if self.high_card == 14 and self.low_card == 10:
                    return "Royal Flush", 10000
                # straight flush
                return "Straight Flush", 8999 + self.low_card
            # flush
            return "Flush", 6000 + sum(10**k * x
                                       for k, x in zip([1, -1, -2, -3], self.values))
        # straight
        elif self.straight():
            if self.high_card == 14 and self.low_card == 2:
                return "Straight", 5000
            return "Straight", 4999 + self.low_card
        # fullhouse
        elif self.fullhouse():
            triple, pair = self.values_counter.most_common(2)
            return "Full House", 7000 + 10 * triple[0] + pair[0]
        # two pair
        candidate1, candidate2, *rest = self.values_counter.most_common()
        rest = sorted(r[0] for r in rest)
        if candidate1[1] == candidate2[1] == 2:
            c0, c1 = sorted([candidate1[0], candidate2[0]])
            return "Two Pairs", 3000 + 10* c0 + c1 + sum(10**k * x
                                                         for k, x in zip([-1, -2], rest))
        # quad
        candidate, *rest = self.values_counter.most_common()
        rest = sorted(r[0] for r in rest)
        if candidate[1] == 4:
            return "Quad", 8000 + 10 * candidate[0] + rest[0]
        # triple
        elif candidate[1] == 3:
            return "Triple", 4000 + 10*candidate[0] + rest[0] + .1*rest[1]
        # pair
        elif candidate[1] == 2:
            return "Pair", 2000 + 10*candidate[0] + rest[0] + .1*rest[1] + .01*rest[2]
        # highcard
        return "High Card", self.high_card

The difference function is taken from the more_itertools package. A collections.Counter object is exactly what it says. If you pass it an iterable it will count how often each object appears and it has some nice methods like most_common which returns tuples of element, count, sorted decreasingly by count.

Now that we have that the main loop becomes quite a bit easier:

from itertools import zip_longest, islice, product
from random import sample

def grouper(iterable, n, fillvalue=None):
    "Collect data into fixed-length chunks or blocks"
    # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx"
    args = [iter(iterable)] * n
    return zip_longest(*args, fillvalue=fillvalue)

if __name__ == "__main__":
    deck = [Card(value, suit) for value, suit in product(range(2, 15), "♥♠♣♦")]
    rounds = int(input("How many rounds? "))
    players = int(input("How many players (max 10)? "))
    show_strength = input("Show result each round? ").lower()[0] == "y"
    assert players <= 10
    hand_occurrence = Counter()

    for _ in range(rounds):
        hands = [Hand(cards) for cards in grouper(sample(deck, players * 5), 5)]
        evaluated_hands = [hand.evaluate() for hand in hands]
        hand_occurrence += Counter(hand[0] for hand in evaluated_hands)
        if show_strength:
            strongest = max(evaluated_hands, key=lambda hand: hand[1])
            print("Strongest hand:", strongest)
    print("Statistics:", hand_occurrence.most_common())

The grouper function which I used to split the hands into groups of 5 cards each is from the itertools recipes.

I did not bother to use nicer user input functions here, but there are plenty examples on this site on how to do it more foolproof (and keep on asking if the supplied input is somehow wrong).

This code takes about 3.14 s ± 82 ms for 10,000 rounds with 10 players and about 34.4 s ± 483 ms for 100,000 rounds on my machine, without printing the results from each round.

\$\endgroup\$
  • 1
    \$\begingroup\$ Much better!, I may add that those strengths are still magic numbers. Maybe add them as an Enum... \$\endgroup\$ – Ludisposed Feb 25 at 10:20
  • \$\begingroup\$ @Ludisposed: True, there is still some room left for improvement, especially regarding readability. But I think I will stop here with the review. Maybe you want to write an answer taking it further? \$\endgroup\$ – Graipher Feb 25 at 10:24
  • \$\begingroup\$ Thanks for this! It performs much better. \$\endgroup\$ – Alec Alameddine Feb 25 at 12:31
4
\$\begingroup\$

To expand on @Graiphers excellent review,

Adding multiple decks

@Graipher differs from your post as he only uses a single deck, but this can easily be added

deck = [
    Card(value, suit) for value, suit in product(range(2, 15), "♥♠♣♦")
    for _ in range(num_of_decks)
]

Avoid magic numbers

Since a number has no special meaning, and you should avoid dedicating chunks of code to a specific number.

If that number changes you would have to change all occurrences of that number which is error-prone

Secondly and I already stressed this, numbers don;t have meaning variables do!

You can use Enum which is in the standard library to solve this problem

class PossibleHands(Enum):
    ROYAL_FLUSH = 10000,
    WHEEL_STRAIGHT_FLUSH = 8999,
    STRAIGHT_FLUSH = 90000,
    FLUSH = 6000
    ...

And when @Graipher returns the name and the strength value, you could use that Enum value

return "Royal Flush", 10000

would be

return PossibleHands.ROYAL_FLUSH , PossibleHands.ROYAL_FLUSH.value

Validate your user input

Your program will currently break if the user input some wrong values,

ie

How many decks are there? s
...
ValueError: invalid literal for int() with base 10: 's'

Or if you input more players then is possible

How many decks are there? 1
How many players are there (max 10)? 21
...
IndexError: pop from empty list

This can be avoided by using a function to check the input

def get_value(_max, mess):
    while True:
        try:
            v = int(input(mess))
            if (_max is None or v <= _max) and v > 0:
                return v
            else:
                smaller = '' if _max is None else f'and smaller then {_max}'
                print(f'Value should be greater then 0 {smaller}')
        except ValueError:
            print('Please enter an integer')

def main():
    num_of_decks = get_value(None, f'How many decks? ')
    deck = [
        Card(value, suit) for value, suit in product(range(2, 15), "♥♠♣♦")
        for _ in range(num_of_decks)
    ]
    max_players = floor((num_decks*52) / 5)
    num_players = get_value(max_players, f'How many players are there? (max {max_players}) ')
   ...

Now the getting of wrong input is handled correctly

How many decks? s
Please enter an integer
How many decks? 2
How many players are there? (max 20) 22
Value should be greater then 0 and smaller then 20
How many players are there? (max 20) 1

Avoid using global variables

You use global val a few times, and this is a bad habit to get into. Because it will make it unclear where a variable is changed. So it makes tracking down that 1 bug a real pain in the ***

@Graipher solves this by having a Hand class

\$\endgroup\$
  • 1
    \$\begingroup\$ Regarding strengths, there's over 7000 distinct strengths a 5 card hand can have. That's why I didn't set individual strengths for each hand (only a baseline plus the other factors). Or am I not understanding what you're trying to say? \$\endgroup\$ – Alec Alameddine Feb 25 at 12:34
  • \$\begingroup\$ I have removed the sentence, it was more of a self note. And may edit the awnser when I have found a better solution ;) \$\endgroup\$ – Ludisposed Feb 25 at 12:38

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.