1
\$\begingroup\$

The problem can be found here.

The file, poker.txt, contains one-thousand random hands dealt to two players. Each line of the file contains ten cards (separated by a single space): the first five are Player 1's cards and the last five are Player 2's cards. You can assume that all hands are valid (no invalid characters or repeated cards), each player's hand is in no specific order, and in each hand there is a clear winner.

How many hands does Player 1 win?

My code works good and solves the problem within 1.2 seconds. I'd like to improve the readability of my code however. Also, I think there are some better ways of comparing 2 hands in my function def winninghand(), but I do not know how. Furthermore, I feel like the way I sort the file may not be the most efficient/best/optimal.

import os
from collections import Counter
from time import time

cards = ['2','3','4','5','6','7','8','9','T','J','Q','K','A'] #A=14, K=13, Q=12, J=11, T=10
colour = ['C','D','H','S'] #clubs, diamonds, hearts, spades
hands = ['high card','pair','two pair','three of kind','straight','flush','full house','four of kind','straight flush'] #royal flush is not included, since it's a special case of a straight flush

#extracts the file
def fileloc(name,extension):
    cur_path = os.path.dirname(__file__)
    with open(os.path.join(cur_path,'..',name+extension),newline=None) as inputfile:
        document = inputfile.read()
    return document

#function to flatten the list and get a tuple
def flatten(l):
  out = []
  for item in l:
    if isinstance(item, (list, tuple)):
      out.extend(flatten(item))
    else:
      out.append(item)
  return out

#function to determine the winning hand
def winninghand(hand):
    #gets a specific hand of player 1 and of player 2
    player1hand = player1_games[hand]
    player2hand = player2_games[hand]
    #valuelist1 will be an ordered list (high - low) of card values of player 1, valuelist2 of player2
    valuelist1 = []
    valuelist2 = []
    #defining winner variable
    winner = -1
    for i in range(5):
        valuelist1.append(player1hand[i][0])
        valuelist2.append(player2hand[i][0])

    #gives a value to T,J,Q,K,A
    valuelist1= [x.replace('T','10').replace('J','11').replace('Q','12').replace('K','13').replace('A','14') for x in valuelist1]
    valuelist2= [x.replace('T','10').replace('J','11').replace('Q','12').replace('K','13').replace('A','14') for x in valuelist2]
    valuelist1 = [int(i) for i in valuelist1]
    valuelist2 = [int(i) for i in valuelist2]
    valuelist1 = sorted(valuelist1,reverse=True)
    valuelist2 = sorted(valuelist2,reverse=True)

    player1value = valuechecker(player1hand,valuelist1)
    player2value = valuechecker(player2hand,valuelist2)

    #winner = 1 means player 1 won, winner = 0 means player 2 wins
    #first checks if hand value is greater for player 1 or 2
    #if it is a draw, it goes past each card value to see who has got the better hand
    if player1value[0] > player2value[0]:
        winner = 1
    elif player2value[0] > player1value[0]:
        winner = 0
    elif player1value[0] == player2value[0]:
        if player1value[1] > player2value[1]:
            winner = 1
        elif player1value[1] < player2value[1]:
            winner = 0
        elif player1value[1] == player2value[1]:
            if player1value[2] > player2value[2]:
                winner = 1
            elif player1value[2] < player2value[2]:
                winner = 0
            elif player1value[2] == player2value[2]:
                if player1value[3] > player2value[3]:
                    winner = 1
                elif player1value[3] < player2value[3]:
                    winner = 0
                elif player1value[3] == player2value[3]:
                    if player1value[4] > player2value[4]:
                        winner = 1
                    elif player1value[4] < player2value[4]:
                        winner = 0
                    elif player1value[4] == player2value[4]:
                        #it was defined in the problem statement that there would always be one winner.
                        raise ValueError("There should be a winner") 

    return winner

#checking the value of a hand
def valuechecker(list,vallist):
    #variables for
    #occs : occurrences of cards
    #mostcommonfreq : amount of occurrences of highest occuring card value
    #nextcommonfreq : amount of occurrences of second highest occuring card value
    #mostcommon : highest occurring card value
    #nextcommon : second highest occurring card value
    #colourtest : checks if all colours are the same
    #consectest : checks if numbers are consecutive
    occs = maxoccurences(vallist)
    mostcommonfreq = occs[0][1]
    nextcommonfreq = occs[1][1]
    mostcommon = occs[0][0]
    nextcommon = occs[1][0]
    colourtest = samecolour(list)
    consectest = consecutive(vallist)

    #check flush
    if colourtest==True and consectest==False:
        val = [hands.index('flush')] + [i for i in vallist]

    #check straight
    elif colourtest==False and consectest==True:
        maximum = max(vallist)
        minimum = min(vallist)
        if maximum==14 and minimum == 2:
            val = [hands.index('straight')] + [i for i in vallist]
            val.remove(14)
            val = vals.append(1)
        else:
            val = [hands.index('straight')] + [i for i in vallist]

    #check straight flush
    elif colourtest==True and consectest==True:
        val = [hands.index('straight flush')] + [i for i in vallist]

    #check pair
    elif(mostcommonfreq == 2 and nextcommonfreq != 2):
        vallist.remove(mostcommon)
        vallist.remove(mostcommon)
        vallist.insert(0,mostcommon)
        vallist.insert(0,mostcommon)
        val = [hands.index('pair')] + [i for i in vallist]

    #check 2 pair
    elif(mostcommonfreq == 2 and nextcommonfreq == 2):
        vallist.remove(mostcommon)
        vallist.remove(mostcommon)
        vallist.remove(nextcommon)
        vallist.remove(nextcommon)
        if mostcommon>nextcommon:
            vallist.insert(0,nextcommon)
            vallist.insert(0,nextcommon)
            vallist.insert(0,mostcommon)
            vallist.insert(0,mostcommon)
        else:
            vallist.insert(0,mostcommon)
            vallist.insert(0,mostcommon)
            vallist.insert(0,nextcommon)
            vallist.insert(0,nextcommon)
        val = [hands.index('two pair')] + [i for i in vallist]

    #check 3 of kind
    elif(mostcommonfreq == 3 and nextcommonfreq != 2):
        vallist.remove(mostcommon)
        vallist.remove(mostcommon)
        vallist.remove(mostcommon)
        vallist.insert(0,mostcommon)
        vallist.insert(0,mostcommon)
        vallist.insert(0,mostcommon)
        val = [hands.index('three of kind')] + [i for i in vallist]

    #check full house
    elif(mostcommonfreq == 3 and nextcommonfreq == 2):
        vallist.remove(nextcommon)
        vallist.remove(nextcommon)
        vallist.append(nextcommon)
        vallist.append(nextcommon)
        val = [hands.index('full house')] + [i for i in vallist]

    #check 4 of kind
    elif(mostcommonfreq == 4):
        vallist.remove(mostcommon)
        vallist.remove(mostcommon)
        vallist.remove(mostcommon)
        vallist.remove(mostcommon)
        vallist.insert(0,mostcommon)
        vallist.insert(0,mostcommon)
        vallist.insert(0,mostcommon)
        vallist.insert(0,mostcommon)        
        val = [hands.index('four of kind')] + [i for i in vallist]

    #check high card
    elif  vallist[0] != vallist[1] != vallist[2] != vallist[3] != vallist[4]:
        val = [hands.index('high card')]+[i for i in vallist]

    return val

#checking whether a hand is a straight -- checking to see if the numbers are in consecutive order
def consecutive(vallist):
    #takes the max and min value of the hand, this is used to check whether it has an Ace, for bottom or top straight
    maxim = max(vallist)
    minim = min(vallist)
    #if max is Ace, and min is 2, then perhaps it is a bottom straight, check first
    if maxim == 14 and minim==2:
        vallist.remove(14)
        vallist.append(1)
        if (vallist[0]==vallist[1]+1==vallist[2]+2==vallist[3]+3==vallist[4]+4):
            return True
        else: 
            #making sure the list is restored to its original state
            vallist.remove(1)
            vallist.insert(0,14)
            return False
    elif (vallist[0]==vallist[1]+1==vallist[2]+2==vallist[3]+3==vallist[4]+4):
            return True 
    else: return False

#checking whether a hand is a flush -- checking to see if the suits are the same
def samecolour(list):
    if list[0][1] == list[1][1] == list[2][1] == list[3][1] == list[4][1]:
        return True
    else: return False

#checking how often a certain card value appears
#used to check for pair, 2 pair, 3 of kind, full house, 4 of kind
def maxoccurences(vallist):
    occu = Counter(vallist).most_common(2)
    return occu


#importing the file with all pokerhands, and turning it into a list with which can be worked
s= time()
gamesPlayed = []
document =  fileloc('poker','.txt')
document = document.replace(' ','').replace('\n','').replace('\r','')
for line in document:
    gamesPlayed.append(line.split(','))

#each character is its own list, so I make a tuple in a list, where each tuple describes a card
gamesPlayed = list(zip(*[iter(flatten(gamesPlayed))]*2))

player1_games = []
startwindow1 = 0
endwindow1 = 5

#extracting only the hands that player 1 had
while endwindow1 <= len(gamesPlayed):
    handList = []
    if endwindow1 > len(gamesPlayed): break
    else: 
        player1_games.append(gamesPlayed[startwindow1:endwindow1])
        startwindow1 += 10
        endwindow1 += 10

player2_games = []
startwindow2 = 5
endwindow2 = 10

#extracting only the hands that player 2 had
while endwindow2 <= len(gamesPlayed):
    handList = []
    if endwindow2 > len(gamesPlayed): break
    else: 
        player2_games.append(gamesPlayed[startwindow2:endwindow2])
        startwindow2 += 10
        endwindow2 += 10

#how many games did player 1 and player 2 win
player1_wins = 0
player2_wins = 0    
totalgames = len(player1_games)

for i in range(len(player1_games)):
    if winninghand(i) == 1:
        player1_wins += 1
    elif winninghand(i) == 0:
        player2_wins += 1

print(player1_wins,player2_wins, time()-s)
Improve this question
\$\endgroup\$
1
  • \$\begingroup\$ See this answer for some ways to reduce the amount of code. \$\endgroup\$
    – Gareth Rees
    Jun 7, 2016 at 15:38

1 Answer 1

Reset to default
2
\$\begingroup\$

A partial answer:

In your function winninghand, you can replace the long lines of nested ifs with:

def comparehands(player1value,player2value):
    for p1, p2 in zip(sorted(player1value, reverse=True), sorted(player2value, reverse=True)):
        if p1 == p2:
            continue
        return p1 > p2
    raise ValueError("There should be a winner")

This will also return 1 if player 1 wins and 0 if player 2 wins.

For constants it is customary to use ALL_CAPS:

CARDS = ['2','3','4','5','6','7','8','9','T','J','Q','K','A']
COLOUR = ['C','D','H','S']

Instead of building a valuelist with fancy replaces use the fact that you already defined an order in CARDS:

player1values = [CARDS.index(card[0]) for card in player1_games[hand]]
player2values = [CARDS.index(card[0]) for card in player2_games[hand]]

This allows you to compactify the function winninghand greatly:

def get_values(playerhand):
    return sorted([CARDS.index(card[0]) for card in playerhand], reverse=True)

def winninghand(hand):
    playerhands = (player1_games[hand], player2_games[hand])
    p1values, p2values = map(get_values, playerhands)
    for p1, p2 in zip(p1values, p2values):
        if p1 == p2:
            continue
        return p1 > p2
    raise ValueError("There should be a winner")

The part of calling valuechecker is still missing in there.

You should almost never use this:

if colourtest==True and consectest==False:
    val = [hands.index('flush')] + [i for i in vallist]

Instead use the fact that colourtest and consectest are already boolean:

if colourtest and not consectest:
    # Do stuff

I would nest these ifs somewhat:

#check straight flush, flush and straight
if colourtest:
    if consectest:
        # straight flush
    else:
        # flush
elif consectest:
    # straight

In samecolour:

def samecolour(list):
    return len(set(card[1] for card in list)) == 1

In the original samecolour, as well as in other places, you are using this as the last line in your function:

else: return False

Just use return False, because if you get to that line, False is the only option left

Improve this answer
\$\endgroup\$

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.