3
\$\begingroup\$

I recently started learning Python and the first program I've made is a program to determine the best 5 card hand from seven total cards and then to be able to compare two different hands to determine which would win. I just wanted to ask if you would be able to have a look at it and give me any thoughts on how I could improve either this program or just my coding in general. I'll put one of the code blocks down below and link to the full program.

Link to full program.

def player_hand_value_check(hand):
    #to keep checking until the highest hand is found
    has_hand=False
    #to assign numerical value to hand so it can be compared in other function
    player_hand_type=""
    #Sorting hand by value
    hand.sort(key=lambda hand:hand.value)
    global player_hand_value, player_hand, player_hand_string
    
    
    #putting
    while has_hand==False:    
        #straight/royal flush check
        straight_flush_hand=[]
        straight_flush_suit_list=[]
        #getting counter of suits in hand
        for cards in hand:
            straight_flush_suit_list.append(cards.suit)
        straight_flush_suit_counter=Counter(straight_flush_suit_list)
        #single suit hand
        for cards in hand:
            if straight_flush_suit_counter[cards.suit]>=5:
                straight_flush_hand.append(cards)
            #royal flush check, using try in case flush_hand isn't long enough
        
        straight_flush_hand.sort(key=lambda straight_flush_hand:straight_flush_hand.value)
        try:
            if straight_flush_hand[-5].value==10 and straight_flush_hand[-4].value==11 and straight_flush_hand[-3].value==12 and straight_flush_hand[-2].value==13 and straight_flush_hand[-1].value==14:
                player_hand_type="Royal Flush"
                player_hand_value=hand_rankings[player_hand_type]
                #pop so it only displays appropriate cards
                if len(straight_flush_hand)==6:
                    straight_flush_hand.pop(0)
                elif len(straight_flush_hand)==7:
                    straight_flush_hand.pop(0)
                    straight_flush_hand.pop(0)
                else:
                    pass
                has_hand=True
                player_hand=straight_flush_hand
                global player_has_royal_flush
                player_has_royal_flush=True
                player_hand_string=f"Royal Flush-{straight_flush_hand[0].suit}"
                
                break
        except IndexError:
            pass
        else:
            pass
        #high end straight flush
        try:
            if straight_flush_hand[-1].value==straight_flush_hand[-2].value+1 and straight_flush_hand[-2].value==straight_flush_hand[-3].value+1 and straight_flush_hand[-3].value==straight_flush_hand[-4].value+1 and straight_flush_hand[-4].value==straight_flush_hand[-5].value+1:
                player_hand_type="Straight Flush"
                player_hand_value=hand_rankings[player_hand_type]
                #pop so it only displays appropriate cards
                if len(straight_flush_hand)==6:
                    straight_flush_hand.pop(0)
                elif len(straight_flush_hand)==7:
                    straight_flush_hand.pop(0)
                    straight_flush_hand.pop(0)
                #change global variables for opponent comparison function
                global player_has_straight_flush, player_straight_flush_value
                player_has_straight_flush=True
                player_straight_flush_value=straight_flush_hand[-1].value
                has_hand=True
                player_hand=straight_flush_hand
                player_hand_string=f"Straight Flush-{straight_flush_hand[-1].rank} High-{straight_flush_hand[-1].suit}"
                break
        except IndexError:
            pass
        else:
            pass
        #mid range straight flush
        try:
            if straight_flush_hand[-2].value==straight_flush_hand[-3].value+1 and straight_flush_hand[-3].value==straight_flush_hand[-4].value+1 and straight_flush_hand[-4].value==straight_flush_hand[-5].value+1 and straight_flush_hand[-5].value==straight_flush_hand[-6].value+1:
                player_hand_type="Straight Flush"
                player_hand_value=hand_rankings[player_hand_type]
                #pop so it only displays appropriate cards
                if len(straight_flush_hand)==6:
                    straight_flush_hand.pop(-1)
                elif len(straight_flush_hand)==7:
                    straight_flush_hand.pop(-1)
                    straight_flush_hand.pop(0)
                player_has_straight_flush=True
                player_straight_flush_value=straight_flush_hand[-2].value
                has_hand=True
                player_hand=straight_flush_hand
                player_hand_string=f"Straight Flush-{straight_flush_hand[-2].rank} High-{straight_flush_hand[-2].suit}"
                break
        except IndexError:
            pass
        else:
            pass
        #low range straight flush
        try:
            if straight_flush_hand[-3].value==straight_flush_hand[-4].value+1 and straight_flush_hand[-4].value==straight_flush_hand[-5].value+1 and straight_flush_hand[-5].value==straight_flush_hand[-6].value+1 and straight_flush_hand[-6].value==straight_flush_hand[-7].value+1:
                player_hand_type="Straight Flush"
                player_hand_value=hand_rankings[player_hand_type]
                #pop so it only displays appropriate cards
                if len(straight_flush_hand)==6:
                    straight_flush_hand.pop(-1)
                elif len(straight_flush_hand)==7:
                    straight_flush_hand.pop(-1)
                    straight_flush_hand.pop(-1)
                player_has_straight_flush=True
                player_straight_flush_value=straight_flush_hand[-3].value
                has_hand=True
                player_hand=straight_flush_hand
                player_hand_string=f"Straight Flush-{straight_flush_hand[-3].rank} High-{straight_flush_hand[-3].suit}"
                break
        except IndexError:
            pass
        else:
            pass
        #ace low straight flush
        try:
            if straight_flush_hand[-1].value==14 and straight_flush_hand[0].value==2 and straight_flush_hand[1].value==3 and straight_flush_hand[2].value==4 and straight_flush_hand[3].value==5: 
                player_hand_type="Straight Flush"
                player_hand_value=hand_rankings[player_hand_type]
                #pop so it only displays appropriate cards
                if len(straight_flush_hand)==6:
                    straight_flush_hand.pop(-2)
                elif len(straight_flush_hand)==7:
                    straight_flush_hand.pop(-2)
                    straight_flush_hand.pop(-2)
                player_has_straight_flush=True
                player_straight_flush_value=straight_flush_hand[3].value
                has_hand=True
                player_hand=straight_flush_hand
                player_hand_string=f"Straight Flush-{straight_flush_hand[3].rank} High-{straight_flush_hand[3].suit}"
                break
        except IndexError:
            pass
        else:
            pass
        #four of a kind, sort by rank, count rank, if 4 in rank, append to new list, add all others to new list, sort that list and attach [-1] to four of a kind hand and player kicker is also[-1]
        hand.sort(key=lambda hand:hand.value)
        four_of_a_kind_hand=[]
        four_of_a_kind_kickers=[]
        four_kind_list=[]
        for cards in hand:
            four_kind_list.append(cards.value)
        #compare to cards in hand
        four_kind_value_counter=Counter(four_kind_list)
        #assign four of a kind and kickers to seperate hand
        for cards in hand:
            if four_kind_value_counter[cards.value]<4:
                four_of_a_kind_kickers.append(cards)
            elif four_kind_value_counter[cards.value]==4:
                four_of_a_kind_hand.append(cards)
            else:
                pass
                #check for length of four_of_a_kind_hand
        if len(four_of_a_kind_hand)>0:
            player_hand_type="Four of a Kind"
            player_hand_value=hand_rankings[player_hand_type]
            #change global variables for opponent comparison function
            global player_has_four_of_a_kind, player_four_of_a_kind_kicker,player_four_of_a_kind_value
            #sorting kickers list and appending -1 value
            four_of_a_kind_kickers.sort(key=lambda hand:hand.value)
            four_of_a_kind_hand.append(four_of_a_kind_kickers[-1])
            player_has_four_of_a_kind=True
            player_four_of_a_kind_value=four_of_a_kind_hand[0].value
            player_four_of_a_kind_kicker=four_of_a_kind_kickers[-1].value
            has_hand=True
            player_hand=four_of_a_kind_hand
            player_hand_string=f"Four of a Kind-{four_of_a_kind_hand[0].rank}s"
            break
            #probably not needed but just for readability
        else: 
            pass
        #full house check
        #sorted by rank, values added to test list, if value counter is 3 added to full house 3, if 2 added to full house pair, sorted then for loop using -1 value
        full_house_list=[]
        full_house_threes=[]
        full_house_pairs=[]
        full_house_hand=[]
            
        for cards in hand:
            full_house_list.append(cards.value)
        #not sure if its a problem to reuse value counter name
        full_house_value_counter=Counter(full_house_list)
        #for loop to add to threes and pairs list
        for cards in hand:
            if full_house_value_counter[cards.value]==3:
                full_house_threes.append(cards)
            elif full_house_value_counter[cards.value]==2:
                full_house_pairs.append(cards)
            else:
                pass
            #in case multiple 3 will append lower 3 to pair list
        if len(full_house_threes)>3:
            full_house_threes.sort(key=lambda full_house_threes:full_house_threes.value)
            full_house_pairs.append(full_house_threes[0])
            full_house_pairs.append(full_house_threes[1])
        else:
            pass
                
            #if 3 list and pair list aren't empty then full house is true
        if len(full_house_threes)>0 and len(full_house_pairs)>0: 
            full_house_threes.sort(key=lambda full_house_threes:full_house_threes.value)
            full_house_pairs.sort(key=lambda full_house_pairs:full_house_pairs.value)
            full_house_hand.append(full_house_threes[-1])
            full_house_hand.append(full_house_threes[-2])
            full_house_hand.append(full_house_threes[-3])
            full_house_hand.append(full_house_pairs[-1])
            full_house_hand.append(full_house_pairs[-2])
            player_hand_type="Full House"
            player_hand_value=hand_rankings[player_hand_type]
            global player_has_full_house, player_full_house_value, player_full_house_kicker
            player_has_full_house=True
            player_full_house_value=full_house_threes[-1].value
            player_full_house_kicker=full_house_pairs[-1].value
            has_hand=True
            player_hand=full_house_hand
            player_hand_string=f"Full House-{full_house_threes[-1].rank}s over {full_house_pairs[-1].rank}s"
            break
        else:
            #problem with flush seems to start here
            pass
        #Flush check, should be similar to royal flush
        #finally fixed, issue was try except for straight flush didn't have an else statement
        flush_hand=[]
        flush_suit_list=[]
        for cards in hand:
            flush_suit_list.append(cards.suit)
        flush_suit_counter=Counter(flush_suit_list)
        #single suit hand
        for cards in hand:
            if flush_suit_counter[cards.suit]>=5:
                flush_hand.append(cards)
        #checking for flush    
        if len(flush_hand)>=5:
            flush_hand.sort(key=lambda flush_hand:flush_hand.value)
            #popping flush_hand appropriately
            if len(flush_hand)==6:
                flush_hand.pop(0)
            elif len(flush_hand)==7:
                flush_hand.pop(0)
                flush_hand.pop(0)
            else:
                pass
            player_hand_type="Flush"
            player_hand_value=hand_rankings[player_hand_type]
            flush_hand.sort(key=lambda flush_hand:flush_hand.value)
            global player_has_flush, player_flush_value
            player_has_flush=True
            player_flush_value=[flush_hand[-1].value,flush_hand[-2].value,flush_hand[-3].value,flush_hand[-4].value,flush_hand[-5].value,]
            has_hand=True
            player_hand=flush_hand
            player_hand_string=f"{flush_hand[-1].suit} Flush-{flush_hand[-1].rank}"
            break
                            
        else:
            pass
        
        #straight check
        #remove code isn't working but straight works with no double ranks
        straight_hand=[]
        for cards in hand:
            straight_hand.append(cards)
        straight_list=[]
        straight_counter=Counter(straight_list)
        for cards in straight_hand:
            straight_list.append(cards.rank)
        straight_counter=Counter(straight_list)
        for cards in straight_hand:
            if straight_counter[cards.rank]>1:
                straight_hand.remove(cards)
        straight_hand.sort(key=lambda straight_hand:straight_hand.value)
        #high end straight, code the same as for straight flush
        try:
            if straight_hand[-1].value==straight_hand[-2].value+1 and straight_hand[-2].value==straight_hand[-3].value+1 and straight_hand[-3].value==straight_hand[-4].value+1 and straight_hand[-4].value==straight_hand[-5].value+1:
                    player_hand_type="Straight"
                    player_hand_value=hand_rankings[player_hand_type]
                    #pop so it only displays appropriate cards
                    if len(straight_hand)==6:
                        straight_hand.pop(0)
                    elif len(straight_hand)==7:
                        straight_hand.pop(0)
                        straight_hand.pop(0)
                    else:
                        pass
                    global player_has_straight, player_straight_value
                    player_has_straight=True
                    player_straight_value=straight_hand[-1].value
                    has_hand=True
                    player_hand=straight_hand
                    player_hand_string=f"Straight-{straight_hand[-1].rank} High"
                    break
        except IndexError:
            pass
        else:
            pass
        #mid range straight,
        try:
            if straight_hand[-2].value==straight_hand[-3].value+1 and straight_hand[-3].value==straight_hand[-4].value+1 and straight_hand[-4].value==straight_hand[-5].value+1 and straight_hand[-5].value==straight_hand[-6].value+1:
                    player_hand_type="Straight"
                    player_hand_value=hand_rankings[player_hand_type]
                    #pop so it only displays appropriate cards
                    if len(straight_hand)==6:
                        straight_hand.pop(-1)
                    elif len(straight_hand)==7:
                        straight_hand.pop(-1)
                        straight_hand.pop(0)
                    else:
                        pass
                    player_has_straight=True
                    player_straight_value=straight_hand[-1].value
                    has_hand=True
                    player_hand=straight_hand
                    player_hand_string=f"Straight-{straight_hand[-1].rank} High"
                    break
        except IndexError:
            pass
        else:
            pass
        #low end straight
        try:
            if straight_hand[-3].value==straight_hand[-4].value+1 and straight_hand[-4].value==straight_hand[-5].value+1 and straight_hand[-5].value==straight_hand[-6].value+1 and straight_hand[-6].value==straight_hand[-7].value+1:
                    player_hand_type="Straight"
                    player_hand_value=hand_rankings[player_hand_type]
                    #pop so it only displays appropriate cards
                    if len(straight_hand)==6:
                        straight_hand.pop(-1)
                    elif len(straight_flush_hand)==7:
                        straight_hand.pop(-1)
                        straight_hand.pop(-1)
                    player_has_straight=True
                    player_straight_value=straight_hand[-1].value
                    has_hand=True
                    player_hand=straight_hand
                    player_hand_string=f"Straight-{straight_hand[-1].rank} High"
                    break
        except IndexError:
            pass
        else:
            pass
        try:
            if straight_hand[-1].value==14 and straight_hand[0].value==2 and straight_hand[1].value==3 and straight_hand[2].value==4 and straight_hand[3].value==5: 
                player_hand_type="Straight"
                player_hand_value=hand_rankings[player_hand_type]
                #pop so it only displays appropriate cards
                if len(straight_hand)==6:
                    straight_hand.pop(-2)
                elif len(straight_hand)==7:
                    straight_hand.pop(-2)
                    straight_hand.pop(-2)
                player_has_straight=True
                player_straight_value=straight_hand[3].value
                has_hand=True
                player_hand=straight_hand
                player_hand_string=f"Straight-{straight_hand[3].rank} High"
                break
        except IndexError:
            pass
        else:
            pass
        #three of a kind check, getting * try and fix, actually was related to straight_hand=hand, obviously they affect each other
        #list to hold all values
        three_kind_list=[]
        #list to find triple cards
        three_kind_threes=[]
        #final hand to return
        three_kind_hand=[]
        #list to hold potential kickers
        three_kind_kickers=[]
        #append to three kind hand
        for cards in hand:
            three_kind_list.append(cards.value)
        #counter for values
        three_kind_value_counter=Counter(three_kind_list)
        for cards in hand:
            if three_kind_value_counter[cards.value]==3:
                three_kind_threes.append(cards)
            elif three_kind_value_counter[cards.value]<3:
                three_kind_kickers.append(cards)
            else:
                pass
        #don't need to check for multiple threes as it should have been caught by full house check
        if len(three_kind_threes)>0:
            three_kind_hand.append(three_kind_threes[0])
            three_kind_hand.append(three_kind_threes[1])
            three_kind_hand.append(three_kind_threes[2])
            three_kind_kickers.sort(key=lambda three_kind_kickers:three_kind_kickers.value)
            three_kind_hand.append(three_kind_kickers[-1])
            three_kind_hand.append(three_kind_kickers[-2])
            player_hand_type="Three of a Kind"
            player_hand_value=hand_rankings[player_hand_type]
            global player_has_three_kind, player_three_kind_value, player_three_kind_kickers
            player_has_three_kind=True
            player_three_kind_value=three_kind_threes[-1].value
            player_three_kind_kickers=[three_kind_kickers[-1].value,three_kind_kickers[-2].value]
            has_hand=True
            player_hand=three_kind_hand
            player_hand_string=f"Three of a kind-{three_kind_threes[0].rank}s"
            break
        else:
            pass
        #two pair check
        #list to hold all values
        two_pair_list=[]
        #list to hold paired hands
        two_pair_pairs=[]
        #list to hold potential kickers
        two_pair_kickers=[]
        #final hand to return
        two_pair_hand=[]
        
        for cards in hand:
            two_pair_list.append(cards.value)
        #counter for values
        two_pair_value_counter=Counter(two_pair_list)
        for cards in hand:
            if two_pair_value_counter[cards.value]==2:
                two_pair_pairs.append(cards)
            elif two_pair_value_counter[cards.value]==1:
                two_pair_kickers.append(cards)
            else:
                pass
        #puts a potential third pair back in with kickers
        if len(two_pair_pairs)>=4:
            two_pair_pairs.sort(key=lambda two_pair_pairs:two_pair_pairs.value)
            if len(two_pair_pairs)>4:
                two_pair_kickers.append(two_pair_pairs[0])
                two_pair_kickers.append(two_pair_pairs[1])
            else:
                pass
            two_pair_kickers.sort(key=lambda two_pair_kickers:two_pair_kickers.value)
            two_pair_hand.append(two_pair_pairs[-1])
            two_pair_hand.append(two_pair_pairs[-2])
            two_pair_hand.append(two_pair_pairs[-3])
            two_pair_hand.append(two_pair_pairs[-4])
            two_pair_hand.append(two_pair_kickers[-1])
            player_hand_type="Two Pair"
            player_hand_value=hand_rankings[player_hand_type]
            global player_has_two_pair, player_two_pair_value, player_two_pair_kicker
            player_has_two_pair=True
            player_two_pair_value=[two_pair_pairs[-1].value,two_pair_pairs[-3].value]
            player_two_pair_kicker=two_pair_kickers[-1].value
            has_hand=True
            player_hand=two_pair_hand
            player_hand_string=f"Two Pair-{two_pair_pairs[-1].rank}s and {two_pair_pairs[-3].rank}s"
            break
        else:
            pass
        #pair check
        #list to hold all values
        pair_list=[]
        #list to hold pair
        pair_pair=[]
        #list to hold kickers
        pair_kickers=[]
        #final hand to return
        pair_hand=[]
        
        #append values to list
        for cards in hand:
            pair_list.append(cards.value)
        #counter for values
        pair_value_counter=Counter(pair_list)
        #append pairs and kickers to appropriate lists, else statement might have been sufficient but I'll use an elif for readability
        for cards in hand:
            if pair_value_counter[cards.value]==2:
                pair_pair.append(cards)
            elif pair_value_counter[cards.value]==1:
                pair_kickers.append(cards)
            else:
                pass
        if len(pair_pair)==2:
            pair_kickers.sort(key=lambda pair_kickers:pair_kickers.value)
            pair_hand.append(pair_pair[0])
            pair_hand.append(pair_pair[1])
            if len(hand)>2:
                pair_hand.append(pair_kickers[-1])
                pair_hand.append(pair_kickers[-2])
                pair_hand.append(pair_kickers[-3])
            player_hand_type="Pair"
            player_hand_value=hand_rankings[player_hand_type]
            global player_has_pair, player_pair_value, player_pair_kickers
            player_has_pair=True
            player_pair_value=pair_pair[-1].value
            if len(hand)>2:
                player_pair_kickers=[pair_kickers[-1].value,pair_kickers[-2].value,pair_kickers[-3].value]
            else:
                pass
            has_hand=True
            player_hand=pair_hand
            player_hand_string=f"A Pair of {pair_pair[-1].rank}s"
            break
        else:
            high_card_hand=[]
            high_card_hand.append(hand[-1])
            high_card_hand.append(hand[-2])
            if len(hand)>2:
                high_card_hand.append(hand[-3])
                high_card_hand.append(hand[-4])
                high_card_hand.append(hand[-5])
            else:
                pass
            high_card_hand.sort(key=lambda high_card_hand:high_card_hand.value)
            player_hand_type="High Card"
            player_hand_value=hand_rankings[player_hand_type]
            global player_has_high_card, player_high_card_value, player_high_card_kickers
            player_has_high_card=True
            player_high_card_value=high_card_hand[-1].value
            if len(hand)>2:
                player_high_card_kickers=[high_card_hand[-2].value,high_card_hand[-3].value,high_card_hand[-4].value,high_card_hand[-5].value]
            else:
                pass
            has_hand=True
            player_hand=high_card_hand
            player_hand_string=f"High Card-{high_card_hand[-1].rank}"
            break
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

The first thing I would do to improve this code is to add a docstring and some unit tests. With the doctest module, the unit tests can be included within the docstring, helping demonstrate the expected result from the function.

For example:

    """
    Return the highest-scoring poker hand from HAND.

    >>> player_hand_value_check(hand("8♥ T♠ J♠ Q♠ K♥ K♠ A♠"))
    straight_flush("A")
    >>> player_hand_value_check(hand("A♠ 2♥ 3♥ 4♥ 5♥ K♣ K♦"))
    straight("5")
    >>> player_hand_value_check(hand("2♠ 2♥ 3♥ 4♥ 5♥ K♣ K♦"))
    two_pair("K", "2", "5")
    ⋮ 
    """

Eliminate global variables - instead of manipulating those in the function, return a suitable object (as in example above) which knows how to compare against other hands and can format itself into a string.

Look at what actions are repeated, and extract those as functions. For example, we could have a function that extracts the cards of the most numerous suit and a function that finds a straight - these can be combined to detect a straight flush.

Similarly, if we have functions to find pairs and trips, we can combine those to find two-pair and full house.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for that. For a docstring would a print statement showing that it's working as normal be enough or does it need more? Also i made a function that automated putting the cards into a list and checking the suits to detect a flush but then what was happening was that if the hand had a flush, it would get stuck in an infinite loop at the royal flush section and I couldn't figure out why, do you have any ideas what might have caused that? Thanks again. \$\endgroup\$
    – Doug160516
    Feb 14, 2022 at 12:08
  • 2
    \$\begingroup\$ I've added an example of using doctest. If you have problems making your code work, then I can't guess what you did wrong. If you can't solve the problem, you should create a minimal reproducible example from that part and ask for help over at Stack Overflow. But try hard to fix it yourself first - start by adding tests of the helper functions, to confirm that they work correctly. \$\endgroup\$ Feb 14, 2022 at 12:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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