Project Euler - Problem 54: testing poker hands

This was a very fun and thought provoking problem, and I'm quite proud of the way I was able to pull it off. I broke it down into 2 parts, testing and comparing.



-
Testing each group of cards for a playable hand. This returns a boolean value depending on if the cards meet the criteria for the
hand. A set of cards may test positive for multiple hands. It is this
reason we start with the highest ranked hand and go down. If player 1
tests positive and the other does not, then player 1 wins that play

-
If both players test positive for the same type of hand, then we must compare both hands. The compare functions test each hand to
determine who wins based on highest value according to the rules of
Poker.

-
If both players do not have a poker and at the end of the tests, do a simple high card comparison to determine winner


Per instructions, this solution does not attempt to verify validity of provided
data. The Card class is a recycled bit of code from an old project of mine that
proved to be a nice little container for card data, making the solution easier.

One thing I think I could improve on is creating a Hand class that holds the cards. Maybe I could then store some data that is frequent (such as my use of groupby in the tests), and make some nifty comparitive operators to compare Hand classes themselves to find out which one outranks the other. Never done something like that before though, don't really know how to start.

Full problem text can be found here.

```
from itertools import groupby
from urllib2 import urlopen
file = 'https://projecteuler.net/project/resources/p054_poker.txt'
data = urlopen(file)

class Card():
suitNames = ('Diamonds', 'Clubs', 'Hearts', 'Spades')
faceNames = ('2', '3', '4', '5', '6', '7', '8', '9', 'Ten', 'Jack', 'Queen', 'King', 'Ace')

def __init__(self, str='AS'):
# start conversion of human-readable string to proper indices
n = len(str)
face, suit = str[0], str[1]

1. Bugs

-
testStraight returns False for an ace-low straight:

>>> hand = sorted(map(Card, '3C AH 4D 2S 5C'.split()), key=lambda x:x.rank)
>>> testStraight(hand)
False

Luckily there are no ace-low straights in the Project Euler data.

-
A comment says "two Royals is not possible" but although this happens to be true of the Project Euler data, this is incorrect in general. If both players get royal flushes then the code fails with:

TypeError: 'NoneType' object is not callable

-
The code compares numbers using is, but this operator is the wrong choice: it tests for object identity, not numeric equality. In CPython, it appears to work for small integers:

>>> 2 + 2 is 4
True

but this is an artefact of the way the implementation caches small int objects, and it fails for larger numbers:

>>> 200 + 200 is 400
False

Compare numbers using the == operator instead.

1. Other review comments

-
The comments at the start of each function could be turned into docstrings, so that you can read them from the interactive interpreter using the help function. For example:

def testFlush(hand):
    """Return True if all cards in hand belong to the same suit."""

-
This line creates an unnecessary list:

set([c.rank for c in hand])

Use a generator expression instead:

set(c.rank for c in hand)

-
This key function appears many times in the code:

key=lambda x: x.rank

This repetition could be avoided by giving the Card class a __lt__ method so that Card objects sort into order by their rank.

-
This expression appears several times:

[len(list(group)) for key, group in groupby(hand, key=lambda x: x.rank)]

It would be better to compute this just once. What this line does is to count how many cards there are of each rank. This could be computed more simply using collections.Counter:

Counter(card.rank for card in hand).values()

-
The function testThreeKind returns True if the hand is three-of-a-kind or if the hand is a full house. Similarly, testPair returns True if the hand is a pair, or two pair, or a full house. So these functions only return the correct result if the higher-quality hands have already been tested and rejected. It is hard to check the correctness of code when it depends on the order of operations like this. It would be better to make these functions test the exact condition that we are interested in. For example, in testPair, instead of the condition 2 in l, we could write:

sorted(l) == [1, 1, 1, 2]

-
There is no need to handle royal flushes as a special case. A royal flush is just an ace-high straight flush, and so it tests and compares under the same rules as an ordinary straight flush.

1. Code length

This code is very long. The longer code is, the harder it is to read, check, and maintain.

A key idea for making this code shorter is to convert each poker hand to a canonical form: a simple data structure that can be compared using Python's built-in comparison operators.

A convenient choice of canonical form consists of a number giving the quality of the hand (high card=1, pair=2, two pair=3, and so on) and a list of the card ranks in descreasing order by frequency and then by rank (using jack=11, queen=12, king=13 and ace=14). So two pairs (tens and sevens) with a king would have the canonical form (3, [10, 7, 13]). This wins against two pairs (tens and sevens) with a queen, because:

(3, [10, 7, 13]) > (3, [10, 7, 12])

but loses to two pairs (jacks and fours) with a nine, because:

(3, [10, 7, 13]) < (3, [11, 4, 9])

Here's an implementation of this idea:

```
from collections import Counter
from enum import IntEnum, unique

@unique
class Quality(IntEnum):
"""Quality of a poker hand. Higher values beat lower values."""
high_card = 1
pair = 2
two_pairs = 3
three = 4
straight = 5
flush = 6
full_house = 7
four = 8
straight_flush = 9

def canonical(hand):
"""Return the canonical form of the poker hand as a pair (q, r) where
q is a value from the Quality enumeration, and r is a list of the
distinct card ranks in the hand (from 1=low ace to 14=high ace),
ordered in descreasing order by frequency and then by rank. These
canonical forms can be compared to see who wins. The hand must be
a sequence of five cards given as two-character strings in the
form 2H, TS, JC etc.

>>> canonical('TD 7H KH TS 7S'.split()) # two pairs (tens and sevens)
(, [10, 7, 13])
>>> canonical('3C AH 4D 2S 5C'.split()) # ace-low straight
(, [5, 4, 3, 2, 1])
>>> canonical('JH 2H JC JS 2D'.split()) # full house (twos and jacks)
(, [11, 2])
>>> canonical('TS 4S 8S QS 5S'.split()) # queen-high flush
(, [12, 10, 8, 5, 4])

"""
flush = len(set(suit for _, suit in hand)) == 1
ranks = sorted('--23456789TJQKA'.find(rank) for rank, _ in hand)
if ranks == [2, 3, 4, 5, 14]: # ac

StackExchange Code Review Q#95059, answer score: 8