8
\$\begingroup\$

I'm writting this poker hand evaluator and it's really coming along. But, as you can see I repeat myself many times and I would love to know what's your thought process when you're repeating yourself? Other than this, it's not finished. I still need to account for high cards etc. But you get the jist of it. I'm really working hard on this one and I hope it shows.

Please be blunt! Take it out on me today. :)

The program is split into two files:

analyzer.py and goodhands.py in that order.

This is the main program for running the analyzer: analyzer.py

"""Poker hand probalities"""

hands = 0
while True:

    from goodhands import *
    import random
    from time import sleep
    from collections import Counter



    def deck():
        """Makes a list of 52 shuffled cards """
        suits = "♣♦♥♠"
        letts = "JQKA"
        specials = [char + suit for suit in suits for char in letts]
        numbers = [str(num) + suit for suit in suits for num in range(2, 11)]
        deck = numbers + specials
        shuffled = random.sample(deck, len(deck))
        return shuffled
    win_deck_check = deck()[:]
    deck = deck()

    def players():
        """Make a dictionary of players to store the values of cards"""
        players = {"p" + str(num):None for num in range(1, 10)}
        return players

    players = players()

    def dealer(deck):
        """Assign one card per player for two turns for a total of two cards"""

        CARD_COUNT = 0
        deck.pop(CARD_COUNT) # Discard the first card of the deck.
        deal = True
        table = {}

        while deal:
            for i in players:
                players[i] = deck[CARD_COUNT] # Assign the first card.
                CARD_COUNT += 1
            if CARD_COUNT == len(players):
                for i in players:
                    players[i] = players[i] + deck[CARD_COUNT] # Assign the second card.
                    CARD_COUNT += 1
            del deck[:18] # Delete player cards from deck.
            for i in range(3):
                table['flop'] = deck[1] + deck[2] + deck[3]
            del deck[:4]# Delete flop from deck.
            table['turn'] = deck[0]
            table['river'] = deck[2]
            del deck[:3] # Delete turn and river from deck.
            break
        return players, table

    plays = dealer(deck)
    # Player hand outcome.

    pone = max(good_hands(plays, 0)) if good_hands(plays, 0) else None
    ptwo = max(good_hands(plays, 1)) if good_hands(plays, 1) else None
    pthree = max(good_hands(plays, 2)) if good_hands(plays, 2) else None
    pfour = max(good_hands(plays, 3)) if good_hands(plays, 3) else None
    pfive = max(good_hands(plays, 4)) if good_hands(plays, 4) else None
    psix = max(good_hands(plays, 5)) if good_hands(plays, 5) else None
    pseven = max(good_hands(plays, 6)) if good_hands(plays, 6) else None
    peight = max(good_hands(plays, 7)) if good_hands(plays, 7) else None
    pnine = max(good_hands(plays, 8)) if good_hands(plays, 8) else None
    outcome = [pone, ptwo, pthree, pfour, pfive, psix, pseven, peight, pnine]

    hands += 1
    if hands == 1:
        break

print(outcome)
print(plays)

This is the analyzer itself: goodhands.py

from collections import Counter


def good_hands(plays, player):

    good_hands = [] # This list will hold all the winning values.
    table = plays[1] # This is the flop, turn and river.
    player_cards = list(plays[0].values()) # List of player_cards
    table_list = "".join(list(table.values())) # list of table values.
    all_cards = [i + table_list for i in player_cards] # Player cards with table values.
    occurrences = list(Counter(all_cards[player]).items()) # Counter is used to make a dict with all the occurences. i.e. "k":2 is two K's.
    count = 0
    letters = "JQKA"
    suits = "♣♦♥♠"

    # Card Counters
    atofive = 0
    twosix = 0
    threeseven = 0
    foureight = 0
    fivenine = 0
    six_ten = 0
    seventoj = 0
    eighttoq = 0
    ninetok = 0
    tentoa = 0
    royal_flush = 0

    # This will replace "1" and "0" with a "10"
    for i in occurrences:
        if "1" in i[0]:
            occurrences.remove(i)
    for i in occurrences:
        if "0" in i[0]:
            occurrences.remove(i)
            occurrences.append(("10",i[1]))
    for i in occurrences:
        if i[0].isdigit() == True: # i[0] is the Suit or the number of a card
            card_value = int(i[0])
        # A - 5 LOW STRAIGHT
        if i[0]:
            if i[0] == "A":
                atofive +=1
        if i[0].isdigit():
                for n in range(2, 6):
                    if int(i[0]) == n:
                        atofive += 1
                    if atofive == 5 and 21 not in good_hands:
                        good_hands.append(21)
        # 2 - 6 LOW STRAIGHT
        if i[0].isdigit() == True:
            for n in range(2, 7):
                if int(i[0]) == n:
                    twosix += 1
                if twosix == 5 and 21.1 not in good_hands:
                    good_hands.append(21.1)

        # 3 - 7 LOW STRAIGHT
        if i[0].isdigit() == True:
            for n in range(3, 8):
                if int(i[0]) == n:
                    threeseven += 1
                if threeseven == 5 and 21.2 not in good_hands:
                    good_hands.append(21.2)
        # 4 - 8 LOW STRAIGHT
        if i[0].isdigit() == True:
            for n in range(4, 9):
                if int(i[0]) == n:
                    foureight += 1
                if foureight == 5 and 21.3 not in good_hands:
                    good_hands.append(21.3)
        # 5 - 9 LOW STRAIGHT
        if i[0].isdigit() == True:
            for n in range(5, 10):
                if int(i[0]) == n:
                    fivenine += 1
                if fivenine == 5 and 21.4 not in good_hands:
                    good_hands.append(21.4)
        # 6 - 10 LOW STRAIGHT
        if i[0].isdigit() == True:
            for n in range(6, 11):
                if int(i[0]) == n:
                    six_ten += 1
                if six_ten == 5and 21.5 not in good_hands:
                    good_hands.append(21.5)
        # 7 - J MID STRAIGHT
        if i[0]:
            if i[0] == "J":
                seventoj +=1
        if i[0].isdigit():
                for n in range(7, 11):
                    if int(i[0]) == n:
                        seventoj += 1
                    if seventoj == 5 and 21.6 not in good_hands:
                        good_hands.append(21.6)
        # 8 - Q MID STRAIGHT
        if i[0]:
            if i[0] == "J":
                eighttoq +=1
            if i[0] == "Q":
                eighttoq +=1
        if i[0].isdigit():
                for n in range(8, 11):
                    if int(i[0]) == n:
                        eighttoq += 1
                    if eighttoq == 5 and 21.7 not in good_hands:
                        good_hands.append(21.7)
        # 9 - k HIGH STRAIGHT
        if i[0]:
            if i[0] == "J":
                ninetok +=1
            if i[0] == "Q":
                ninetok +=1
            if i[0] == "K":
                ninetok +=1
        if i[0].isdigit():
                for n in range(9, 11):
                    if int(i[0]) == n:
                        ninetok += 1
                    if ninetok == 5 and 21.8 not in good_hands:
                        good_hands.append(21.8)
        # 10 - A HIGHEST STRAIGHT
        if i[0]:
            if i[0] == "10":
                tentoa += 1
            if i[0] == "J":
                tentoa +=1
            if i[0] == "Q":
                tentoa +=1
            if i[0] == "K":
                tentoa +=1
            if i[0] == "A":
                tentoa += 1
            if tentoa == 5 and 21.9 not in good_hands:
                good_hands.append(21.9)

        if i[0].isdigit() == True and i[1] == 2: # i[1] are the instances
            count += 1
            if card_value <= 6:
                good_hands.append(0) # Low Pair
            if card_value > 6 and card_value <= 9:
                good_hands.append(1) # Mid Pair
            if card_value == 10:
                good_hands.append(2) # Mid-High Pair
            # Two Pair      
            if i[0].isdigit() == True and count == 2:
                good_hands.append(9) 

        # Letters Pairs
        if i[0] in letters:
            if i[1] == 2:
                if i[0] == "J":
                    good_hands.append(3) # Mid-High
                if i[0] == "Q":
                    good_hands.append(4) # High Pair
                if i[0] == "K":
                    good_hands.append(5) # High Pair
                if i[0] == "A":
                    good_hands.append(10) # Highest Pair
            # Letter Trips
            if i[1] == 3:
                if i[0] == "J":
                    good_hands.append(11.90) 
                if i[0] == "Q":
                    good_hands.append(11.91)
                if i[0] == "K":
                    good_hands.append(11.92)
                if i[0] == "A":
                    good_hands.append(11.93) 
                # Letter Quads
                if i[1] == 4:
                    if i[0] == "J":
                        good_hands.append(25.90) 
                    if i[0] == "Q":
                        good_hands.append(25.91)
                    if i[0] == "K":
                        good_hands.append(25.92)
                    if i[0] == "A":
                        good_hands.append(25.93) 
                    # Letter QUADS VALUE OF **** 20 *****
        # Num Trips
        if i[0].isdigit() == True and i[1] >= 3:
            if [0] == "2":
                good_hands.append(11.1)
            if [0] == "3":
                good_hands.append(11.2)
            if [0] == "4":
                good_hands.append(11.3)
            if [0] == "5":
                good_hands.append(11.4)
            if [0] == "6":
                good_hands.append(11.5)
            if [0] == "7":
                good_hands.append(11.6)
            if [0] == "8":
                good_hands.append(11.7)
            if [0] == "9":
                good_hands.append(11.8)
        # Number Quads
            if i[1] == 4:
                if [0] == "2":
                    good_hands.append(25.1)
                if [0] == "3":
                    good_hands.append(25.2)
                if [0] == "4":
                    good_hands.append(25.3)
                if [0] == "5":
                    good_hands.append(25.4)
                if [0] == "6":
                    good_hands.append(25.5)
                if [0] == "7":
                    good_hands.append(25.6)
                if [0] == "8":
                    good_hands.append(25.7)
                if [0] == "9":
                    good_hands.append(25.8)
        # Full House
        pairs = [0,1,2,3,4,5,10]

        trips = [11.1, 11.2, 11.3, 11.4, 
                 11.5, 11.6, 11.7, 11.8, 
                 11.9, 11.91, 11.92, 11.93, 11.93]
        for p in pairs:
            if p in good_hands:
                for t in trips:
                    if t in good_hands and 26 not in good_hands:
                        good_hands.append(26)

        # Flush
        if i[0].isdigit() == False and i[1] == 5:
            good_hands.append(23) 

        # Straight Flushes
        # Royal Flush
        p1_cards = all_cards[0]
        royal_count = 0
        for suit in suits:
            if "10" + suit in p1_cards:
                royal_count += 1
                if "J" + suit in p1_cards:
                    royal_count += 1
                    if "Q" + suit in p1_cards:
                        royal_count += 1
                        if "K" + suit in p1_cards:
                            royal_count += 1
                            if "A" + suit in p1_cards:
                                royal_count += 1
                                if royal_count == 5:
                                    print("ROYAL FLUSH!!!!")
                                    good_hands.append(100)







    return good_hands

I'm more than thankful for the advice because I really need it. I've been coding 'seriously' for about two months after one year off.

\$\endgroup\$
9
\$\begingroup\$

Note: general observation before starting to say anything about code written here, my humble not important in any way opinion here is, this might be too far advanced than your current level of coding skills given by the way you write your code I suggest that you write simpler codes first, get used to what's wrong and what's right and then scale your programs as you go through your learning journey. The amount of mistakes here is insane. Even if this code does the job you created for, this might be the machine understands the instructions you presented, however a code is meant to be interpreted by machines and read by human beings and it's ultra-hard to make sense of your code.

A better example: Find and display best Poker hand I suggest you check this if your code is meant to assess a poker hand, might give you some idea on other approaches to write a similar code.

As I remember mentioning earlier in your previous card game My second game: War Card game V.1 to refer to PEP0008 when writing your code https://www.python.org/dev/peps/pep-0008/

  • An infinite loop full of imports and function definitions: analyzer.py and goodhands.py (both contain the same approach)

    while True:
        from goodhands import *
        import random 
    

    Let me explain the main structure of your program: you keep defining your functions each time you go through the loop and keep importing things as well. This is a terribly inefficient and poor structure. A loop is meant to automate repetitive tasks. In some cases functions are defined inside loops and since this is not one of them, your functions should be defined outside the loop plus import statements according to PEP0008 should be always at the top of your script.

  • Possible bug: When I run the one you call analyze.py I get this result [21.5, 0, 2, None, 23, 5, None, None, 5] and sometimes a list full of numbers which is inconsistent and very ambiguous to anyone trying to make sense of the code. And this is due to the fact that in your goodhands.py, there are 10 p1 through p10 that have a value or a None value(What is this exactly?)
  • Docstrings and type hints: I will not repeat myself defining what those are(refer to your earlier post where I explained what those are) your function good_hands() is A) Ultra long (250 lines for 1 function) that's a mess, you should break it into several functions doing specific tasks in probably 75% less than this and B)Type hints and docstrings are absent and it's hard for anyone trying understand your code to make conclusions about what this function does and what the parameters plays and player are. They could be strings, lists, dicts ...
  • DRY code: Which stands for DON'T REPEAT YOURSELF! opposite to WET 'write everything twice' There are loops that automate repeating tasks, if you find yourself repeating a code even twice ... think again.

    Terrible repetitions in good_hands():

    for i in occurrences: repeated 3 times

    if i[0].isdigit() == True: repeated 13 times!

  • How to Never Ever do this? It's done in the following way:

    if i[0].isdigit():
        # do 1
        # do 2
        # do 3
        # ...
        # do 13
    

    The if i[0].isdigit(): shows up once and contains 13 things to execute instead of the 13 repetitions.

  • Comparison to True: I also remember mentioning this as well in your previous game, whenever you have a condition, you shouldn't be comparing to True and false if condition == True: is the same as if condition: and if condition == False: is the same as if not condition:

  • Naming: you should Never Ever repeat an identifier name ... also in good_hands() there is a list called good_hands what is happening here is first good_hands was a function and inside its body, it became a list ... this is terribly wrong, maybe it did not produce side effects and maybe it did, I don't know however this is a no no never situation. Same goes for deck = numbers + specials in deck() function, players = {"p" + str(num): None for num in range(1, 10)} in players() function.

  • A dictionary that becomes a list: occurrences = list(Counter(all_cards[player]).items()) Is there any explanation here to why are you listing the Counter() dict?

  • Magic numbers: and the list of magic numbers in your code can never be exhausted ... Examples:

    • lines 17 to 27 in good_hands() what are these variables that have values of 0 (all of them)
    • line 34 if "0" in i[0]: what is '0'?
    • line 45 for n in range(2, 6): what is n? why range(2, 6) why not range(2, 9)?
    • line 48 if atofive == 5 and 21 not in good_hands: what is 5? what is 21?
    • line 52 for n in range(2, 7): what is 2? why 7?
    • And as I said the list is endless, so I won't go through them all but you must've got the point
  • Nested structures: line 237 through 250

    for suit in suits:
        if "10" + suit in p1_cards:
            royal_count += 1
            if "J" + suit in p1_cards:
                royal_count += 1
                if "Q" + suit in p1_cards:
                    royal_count += 1
                    if "K" + suit in p1_cards:
                        royal_count += 1
                        if "A" + suit in p1_cards:
                            royal_count += 1
                            if royal_count == 5:
                                print("ROYAL FLUSH!!!!")
                                good_hands.append(100)
    

    This is seriously terrible: 6 nested ifs? and what for? the conditions do not depend on each other and can be:

    for suit in suits:
        if "10" + suit in p1_cards:
            royal_count += 1
        if "J" + suit in p1_cards:
            royal_count += 1
        if "Q" + suit in p1_cards:
            royal_count += 1
        if "K" + suit in p1_cards:
            royal_count += 1
        if "A" + suit in p1_cards:
            royal_count += 1
        if royal_count == 5:
            print("ROYAL FLUSH!!!!")
            good_hands.append(100)
    
  • False shuffle: I also mentioned this as well in your previous game, shuffled = random.sample(deck, len(deck)) does not shuffle, it creates card duplicates and I remember the set() example I used earlier produced 43 cards instead of 52. To properly shuffle, use random.shuffle(list_name) and after shuffling return the list unless there is no point to shuffle in the first place(which is the case here) because the card selection is already random so there is no point in shuffling the list and as I can see you copied the deck() function I presented in the early review without understanding how it works, never copy a code unless you fully understand it(and copying code is not the best way to become a good programmer).

  • main guard: was also mentioned earlier(please revise the previous feedbacks)

    print(outcome)
    print(plays)
    

    should be inside if __name__ == '__main__': at the end of your script after the elimination of the while loop that continuously defines and imports things in the following structure:

    import thing_one
    import thing_two
    
    
    def func1:
    
    def func2:
    
    def func3:
    
    # ...
    
    if __name__ == '__main__':
        # do things
    
  • Useless code:

     hands += 1
     if hands == 1:
        break
    

    should be just break (given that the while loop is not already useless)

  • Code structure suggestions: Finally a better way to do this(which is an assignment left to you) if you insist on working on something with that level of difficulty, I suggest making the following functions: A) a deck builder B) A poker hand scoring function C)A hand assessor/analyzer/comparison function and you might break these functions down to smaller functions and you go from there.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ They don't call you the bullseye for nothing. Thank you for your constructive criticism. I will re-read the comments on my other codes and really analyze what's wrong with my pov. Your last statement really hit the nail or the bullseye. I literally jumped the gun and started coding like a maniac without dividing my code neatly. And, Bro do you even if? Should be on my forehead for the time being. Thanks again. \$\endgroup\$ – Python Novice Oct 5 '19 at 3:39
  • 1
    \$\begingroup\$ Haha, thanks man i'm not that good, you'll sooner or later get used to how to think more like a computer and for the 'starting to code like a maniac' part, I suggest before you start writing the code to decide on the structure of your program and might change that along the way and avoid repetition and make it as short as possible. \$\endgroup\$ – bullseye Oct 5 '19 at 3:52
  • 2
    \$\begingroup\$ I'll add to the mention of PEP-8: there are two utilities you can install to help with your coding style: flake8 and pylint. Both can be installed with pip install flake8 (or pylint). Both install command line utilities you can run like flake8 myfile.py which will report style and potential code-bug issues with your code. For code to be reviewed, it would be a good idea to clean the code up with one or both of those tools prior to posting -- it eliminates a whole class of complaints from reviewers. \$\endgroup\$ – Austin Hastings Oct 5 '19 at 5:50
  • 1
    \$\begingroup\$ Good points. I'd just like to mentioned two things regarding the "An infinite loop full of imports and function definitions" bit though: afaik, re-importing an already imported module is a no-op. It doesn't actually re-import the module, so there shouldn't be any overhead costs (although, as you said, they should still be at the top regardless for correctness). I also wouldn't say that there's never a reason to define a function in a loop. If the function closes over the loop vars, it may be neater than passing them in. Especially since PEP8 discourages "named lambdas", it may be necessary. \$\endgroup\$ – Carcigenicate Oct 8 '19 at 12:08
  • 1
    \$\begingroup\$ What @Carcigenicate said. Claiming that function definitions should Never be inside a loop Ever is not true in general. Sometimes closures are needed. \$\endgroup\$ – Reinderien Oct 8 '19 at 12:45
5
\$\begingroup\$

Whenever you see large amounts of duplication, you should think about what's different about each piece, and what's the same. Once you see what's different, you can try to "automate" the duplicated bits.

For example, look at this code:

pone = max(good_hands(plays, 0)) if good_hands(plays, 0) else None
ptwo = max(good_hands(plays, 1)) if good_hands(plays, 1) else None
pthree = max(good_hands(plays, 2)) if good_hands(plays, 2) else None
pfour = max(good_hands(plays, 3)) if good_hands(plays, 3) else None
pfive = max(good_hands(plays, 4)) if good_hands(plays, 4) else None
psix = max(good_hands(plays, 5)) if good_hands(plays, 5) else None
pseven = max(good_hands(plays, 6)) if good_hands(plays, 6) else None
peight = max(good_hands(plays, 7)) if good_hands(plays, 7) else None
pnine = max(good_hands(plays, 8)) if good_hands(plays, 8) else None

Every line is nearly identical. The only differences are the second argument passed to good_hands, and the name of the variable that you're creating. You're also calling good_hands twice as often as needed, and that looks like a very expensive function.

This block can be easily automated though. To generate numbers, you can use range, and once you have a range of numbers, you can just loop over them:

outcome = []

for n in range(9):
    # Save the result instead of doing large amounts of work twice per n
    result = good_hands(plays, n)

    play = max(result) if result else None

    outcome.append(play)

Note how there is next to no duplication in this code. It can be improved further though. Whenever you want to turn one sequence into another sequence, you should consider using map (or, in Python specifically, a list comprehension).

That whole original chunk you had can be expressed in two, quite terse lines:

raw_results = [good_hands(plays, n) for n in range(9)]  # Calls good_hands on each n

outcome = [max(result) if result else None for result in raw_results]  # Then finds the max of each (or None if it's falsey)

While this is arguably better, I'm not super happy with it. This really could be one simple line (and avoid the extra overhead) if you didn't need to call max, or you were using Python 3.8. Because you need to call max on a potentially invalid result returned by good_hands, you really need an intermediate variable to avoid calling good_hands twice. I achieved that here by splitting the operation over two list comprehensions. In Python 3.8 though, I'd be able to make use of an assignment expression and create a variable inside the comprehension:

outcome = [max(r) if (r := good_hands(plays, n)) else None) for result in raw_results]

Although this doesn't really read well.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Automate isn't the right verb, rather "factor out" the commonality. \$\endgroup\$ – j4hangir yesterday

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.