Weekend Challenge: Ruby Poker Hand Evaluation

By Posted on

Problem

I’m late to this weekend challenge (sorry), but since it’s all in good fun, I hope that’s ok. I’m no poker player, though, so I may have completely overlooked something.

The Hand class does the evaluation, and calculates (among other things) a score array which is ordered for possible comparison with another poker hand. First item is the hand’s score (0-8), and the following 1-5 items are the tie-breaking values/kickers. E.g. a three-of-a-kind hand might be scored as

[3, 5, 9, 6] # base score, value of tripled card, kicker, kicker

Or, for comparison purposes, consider two two-pair hands

player1.score #=> [2, 7, 5, 3]
player2.score #=> [2, 7, 5, 8]

Both players have two pairs of 7s and 5s, but player 2 wins by having the higher kicker.

The (known and intentional) limitations are:

  • 5 cards per hand only (i.e. no communal cards, etc.)
  • No support for jokers/wildcards
  • No validation of the cards

It does take into account high and low aces when determining straights, but otherwise it’s not terribly flexible. (Of course, you can sidestep the “5 cards only” limitation by just brute-force checking 5-card combinations one at a time using, say, Array#combination, but that’s another story.)

I haven’t looked at how this challenge has been solved in other languages, so perhaps there are some tricks I’m missing. But really, the point was mostly to see how far I could get with a fairly functional approach and array/enumerable methods. The code’s mostly one-line methods, so it went OK, I think.

Haven’t bothered with optimization yet, but (if nothing else) a bunch of values can be memoized with a smattering of ||=. However, I’m more interested in critiques of the overall approach (I just like ? methods, ok?!) and possible alternatives (either overall or for specific parts)

The full code (including tests and more verbose comments) is in this gist; below are the principal classes (see further notes below)

ACE_LOW  = 1
ACE_HIGH = 14

# Use Struct to model a simple Card class
Card = Struct.new :suit, :value

# This class models and evaluates a hand of cards
class Hand
  attr_reader :cards

  RANKS = {
    straight_flush:  8,
    four_of_a_kind:  7,
    full_house:      6,
    flush:           5,
    straight:        4,
    three_of_a_kind: 3,
    two_pair:        2,
    pair:            1
  }.freeze

  def initialize(cards)
    raise ArgumentError unless cards.count == 5
    @cards = cards.freeze
  end

  # The hand's rank as an array containing the hand's
  # type and that type's base score
  def rank
    RANKS.detect { |method, rank| send :"#{method}?" } || [:high_card, 0]
  end

  # The hand's type (e.g. :flush or :pair)
  def type
    rank.first
  end

  # The hand's base score (based on rank)
  def base_score
    rank.last
  end

  # The hand's score is an array starting with the
  # base score, followed by the kickers.
  def score
    [base_score] + kickers
  end

  # Tie-breaking kickers, ordered high to low.
  def kickers
    repeat_values + (aces_low? ? aces_low_values.reverse : single_values)
  end

  # If the hand's straight and flush, it's a straight flush
  def straight_flush?
    straight? && flush?
  end

  # Is a value repeated 4 times?
  def four_of_a_kind?
    repeat_counts.include? 4
  end

  # Three of a kind and a pair make a full house
  def full_house?
    three_of_a_kind? && pair?
  end

  # If the hand only contains one suit, it's flush
  def flush?
    suits.uniq.count == 1
  end

  # This is the only hand where high vs low aces comes into play.
  def straight?
    aces_high_straight? || aces_low_straight?
  end

  # Is a card value repeated 3 times?
  def three_of_a_kind?
    repeat_counts.include? 3
  end

  # Are there 2 instances of repeated card values?
  def two_pair?
    repeat_counts.count(2) == 2
  end

  # Any repeating card value?
  def pair?
    repeat_counts.include? 2
  end

  # Actually just an alias for aces_low_straight?
  def aces_low?
    aces_low_straight?
  end

  # Does the hand include one or more aces?
  def aces?
    values.include? ACE_HIGH
  end

  # The repeats in the hand
  def repeats
    cards.group_by &:value
  end

  # The number of repeats in the hand, unordered
  def repeat_counts
    repeats.values.map &:count
  end

  # The values that are repeated more than once, sorted by
  # number of occurrences
  def repeat_values
    repeated = repeats.map { |value, repeats| [value.to_i, repeats.count] }
    repeated = repeated.reject { |value, count| count == 1 }
    repeated = repeated.sort_by { |value, count| [count, value] }.reverse
    repeated.map(&:first)
  end

  # Values that are not repeated, sorted high to low
  def single_values
    repeats.select { |value, repeats| repeats.count == 1 }.map(&:first).sort.reverse
  end

  # Ordered (low to high) array of card values (assumes aces high)
  def values
    cards.map(&:value).sort
  end

  # Unordered array of card suits
  def suits
    cards.map(&:suit)
  end

  # A "standard" straight, treating aces as high
  def aces_high_straight?
    straight_values_from(values.first) == values
  end

  # Special case straight, treating aces as low
  def aces_low_straight?
    aces? && straight_values_from(aces_low_values.first) == aces_low_values
  end

  # The card values as an array, treating aces as low
  def aces_low_values
    cards.map(&:value).map { |v| v == ACE_HIGH ? ACE_LOW : v }.sort
  end

  private

  # Generate an array of 5 consecutive values
  # starting with the `from` value
  def straight_values_from(from)
    (from...from + 5).to_a
  end
end

Notes and edits

As a rule, I consider aces high (value of 14), and only count them as low (value of 1) when checking for an aces-low straight. That is, an ace Card instance will have a value of 14, but in the context of an aces-low straight, a Hand instance will report it as 1.

Hand and Card instances are considered immutable (though, technically, cards aren’t immutable, since I’m using Struct, but that’s only for the purposes of this challenge; otherwise I’d define a “proper” class)

Looking at the code again, here are my own concerns (beyond the limitations noted above):

  • Some methods return unordered arrays, some sort from high to low, and yet others from low to high. Might be nice to make this more consistent.
  • The straight-checking is pretty naïve: Generate 5 consecutive numbers and see if they match the card values. I considered enumerating the values in various ways instead, but a simple == comparison with a generated array seemed more straightforward than what I could come up with.
  • There’s some repetition required in the RANKS hash keys and the method names, but I found it cleaner than the alternatives I played with.

Solution

I think your example is well structured, I hope my review does it some justice. There are your concerns and some additional points I want to discuss:

You’ll find a working implementation of the following code here

  1. General coding style

    Is nice and consistent. I particularly like your use of question marks to denote methods returning a boolean value.

    • Maybe you could omit braces also from method definitions, since you omit them in method calls where possible.
    • You left RANKS, your attributes and the initializer without documentation. I think particularly RANKS and the initializer would benefit from it.

  2. Repetition in RANKS and instance methods

    I think this is ok, and I couln’t come up with a better or more readable way.

  3. The flush method

    They could be written more elegantly using Enumerable#one?

     # If the hand only contains one suit, it's flush
 def flush?
   suits.uniq.one?
 end

  4. Decision to make cards a Struct

    I like the use of constants in your code to improve readability and maintainability. One thing that caught my eye was that ACE_LOW and ACE_HIGH are global constants, while RANKS is not (which is good). I think this design flaw comes from your decision to make Card a Struct object, and the often deriving lack of logic in these kind of objects. Lets change that and make Card a first class citizen: Your code will benefit.

    class Card
  include Comparable

  attr_reader :suit, :value

  # Value to use as ace low
  ACE_LOW = 1

  # Value to use as ace high
  ACE_HIGH = 14

  # initialize the card with a suit and a value
  def initialize suit, value
    super()
    @suit = suit
    @value = value
  end

  # Return the low card
  def low_card
    ace? ? Card.new(suit, ACE_LOW) : self
  end

  # Return if the card is an ace high
  def ace?
    value == ACE_HIGH
  end

  def ace_low?
    value == ACE_LOW
  end

  # Return if the card has suit spades
  def spades?
    suit == :spades
  end

  # Return if the card has suit diamonds
  def diamonds?
    suit == :diamonds
  end

  # Return if the card is suit hearts
  def hearts?
    suit == :hearts
  end

  # Return if the card has suit clubs
  def clubs?
    suit == :clubs
  end

  # Compare cards based on values and suits
  # Ordered by suits and values - the suits_index will be introduced below
  def <=> other
    if other.is_a? Card
      (suit_index(suit) <=> suit_index(other.suit)).nonzero? || value <=> other.value
    else
      value <=> other
    end
  end

  # Allow for construction of card ranges across suits
  # the suits_index will be introduced below
  def succ
    if ace?
      i = suit_index suit
      Card.new(Deck::SUITS[i + 1] || Deck::SUITS.first, ACE_LOW)
    else
      Card.new(suit, value + 1)
    end
  end

  def successor? other
    succ == other
  end

  def straight_successor? other
    succ === other
  end

  # Compare cards for equality in value
  def == other
    if other.is_a? Card
      value == other.value
    else
      value == other
    end
  end
  alias :eql? :==

  # overwrite hash with value since cards with same values are considered equal
  alias :hash :value

  # Compare cards for strict equality (value and suit)
  def === other
    if other.is_a? Card
      value == other.value && suit == other.suit
    else
      false
    end
  end

  private

  # If no deck, this has to be done with an array of suits
  # gets the suit index
  def suit_index suit
    Deck::SUITS.index suit
  end
end

    This would make the following improvements to the code in Hand possible:

    class Hand

  # ... 

  # Tie-breaking kickers, ordered high to low.
  def kickers
    same_of_kind + (aces_low? ? aces_low.reverse : single_cards)
  end

  # If the hand's straight and flush, it's a straight flush
  def straight_flush?
    straight? && flush?
  end

  # Is a value repeated 4 times?
  def four_of_a_kind?
    same_of_kind? 4
  end

  # Three of a kind and a pair make a full house
  def full_house?
    same_of_kind?(3) && same_of_kind?(2)
  end

  # If the hand only contains one suit, it's flush
  def flush?
    suits.uniq.one?
  end

  # This is the only hand where high vs low aces comes into play.
  def straight?
    aces_high_straight? || aces_low_straight?
  end

  # Is a card value repeated 3 times?
  def three_of_a_kind?
    collapsed_size == 2 && same_of_kind?(3)
  end

  # Are there 2 instances of repeated card values?
  def two_pair?
    collapsed_size == 2 && same_of_kind?(2)
  end

  # Any pair?
  def pair?
    same_of_kind? 2
  end

  def single_cards
    cards.select{|c| cards.count(c) == 1 }
  end

  # Does the hand include one or more aces?
  def aces?
    cards.any? &:ace?
  end

  # Ordered (low to high) array of card values (assumes aces high)
  def values
    cards.map(&:value).sort
  end

  # Unordered array of card suits
  def suits
    cards.map &:suit
  end

  # A "standard" straight, treating aces as high
  def aces_high_straight?
    all_successors? cards.sort_by(&:value)
  end

  # Special case straight, treating aces as low
  def aces_low_straight?
    aces? && all_successors?(aces_low)
  end
  alias :aces_low? :aces_low_straight?

  # The card values as an array, treating aces as low
  def aces_low
    cards.map(&:low_card).sort
  end

  private

  # Are there n cards same of kind?
  def same_of_kind?(n)
    !!cards.detect{|c| cards.count(c) == n }
  end

  # How many cards vanish if we collapse the cards to single values
  def collapsed_size
    cards.size - cards.uniq.size
  end

  # map the cards that are same of kind
  def same_of_kind
    2.upto(4).map{|n| cards.select{|c| cards.count(c) == n }.reverse }.sort_by(&:size).reverse.flatten.uniq
  end

  # Are all cards succeeding each other in value?
  def all_successors?(cards)
    cards.all?{|a| a === cards.last || a.successor?(cards[cards.index(a) + 1]) }
  end

end

    Also

    • it would contain the constants ACE_HIGH and ACE_LOW in Card
    • changing a card would be impossible. Values of cards as Struct in frozen cards array can still be modified since the structs respond to value= and suit=

  5. The Hand initializer

    I think it would be nicer to have cards as a splat argument. To initialize with an array unnecessarily decreases readability. Also, the ArgumentError you raise is not very descriptive, which might lead to some confusion. All in all, this is how my improvements would look like:

    def initialize(*cards)
  raise ArgumentError.new "wrong number of cards (#{cards.count} for 5)" unless cards.count == 5
  @cards = cards.freeze
end

    Depending on context of use, an additional sanity check may also be necessary: Check if you really receive 5 instances of Card.

The following points are suggestions where you could go from here

  1. Make Hand a subclass of Array

    A hand of cards is also an array of cards – The resemblance allows you to subclass Array with Hand. This will allow you to use the Enumerable and Array DSL directly on Hand,
    which could benefit you if you were to take this code any further (Think about a Deck and a Game class)

    Also, the way I’ll do it will give you default sorting which should eliminate your problem with unsorted returns, plus you can call sort for suit-and-value based sorting.

    Additional sanity checks might be in order as soon as you decide to not freeze Hand on initialization:

    • Check if push, unshift, insert and << get an instance of Card as argument
    • Check if push, unshift, insert and << don’t add too many cards to a hand

    So, what does that make possible? Let’s refactor:

    class Hand < Array

  # .. RANKS

  def initialize(*cards)
    raise ArgumentError.new "There must be 5 cards" unless cards.count == 5
    super(cards)
    sort_by! &:value # This will give you a nicely sorted hand by default
    freeze
  end

  # The hand's rank as an array containing the hand's
  # type and that type's base score
  def rank
    RANKS.detect { |method, rank| send :"#{method}?" } || [:high_card, 0]
  end

  # The hand's type (e.g. :flush or :pair)
  def type
    rank.first
  end

  # The hand's base score (based on rank)
  def base_score
    rank.last
  end

  # The hand's score is an array starting with the
  # base score, followed by the kickers.
  def score
    ([base_score] + kickers.map(&:value))
  end

  # Tie-breaking kickers, ordered high to low.
  def kickers
    same_of_kind + (aces_low? ? aces_low.reverse : single_cards.reverse)
  end

  # If the hand's straight and flush, it's a straight flush
  def straight_flush?
    straight? && flush?
  end

  # Is a value repeated 4 times?
  def four_of_a_kind?
    same_of_kind? 4
  end

  # Three of a kind and a pair make a full house
  def full_house?
    same_of_kind?(3) && same_of_kind?(2)
  end

  # If the hand only contains one suit, it's flush
  def flush?
    suits.uniq.one?
  end

  # single cards in the hand
  def single_cards
    select{ |c| count(c) == 1 }.sort_by(&:value)
  end

  # This is the only hand where high vs low aces comes into play.
  def straight?
    aces_high_straight? || aces_low_straight?
  end

  # Is a card value repeated 3 times?
  def three_of_a_kind?
    collapsed_size == 2 && same_of_kind?(3)
  end

  # Are there 2 instances of repeated card values?
  def two_pair?
    collapsed_size == 2 && same_of_kind?(2)
  end

  # Any repeating card value?
  def pair?
    same_of_kind?(2)
  end

  # Does the hand include one or more aces?
  def aces?
    any? &:ace?
  end

  # Ordered (low to high) array of card values (assumes aces high)
  def values
    map(&:value).sort
  end

  # Ordered Array of card suits
  def suits
    sort.map &:suit
  end

  # A "standard" straight, treating aces as high
  def aces_high_straight?
    all?{|card| card === last || card.successor?(self[index(card) + 1]) }
  end
  alias :all_successors? :aces_high_straight?

  # Special case straight, treating aces as low
  def aces_low_straight?
    aces? && aces_low.all_successors?
  end
  alias :aces_low? :aces_low_straight?

  # The card values as an array, treating aces as low
  def aces_low
    Hand.new *map(&:low_card)
  end

  private

  # Are there n cards of the same kind?
  def same_of_kind?(n)
    !!detect{|card| count(card) == n }
  end

  def same_of_kind
    2.upto(4).map{|n| select{|card| count(card) == n }.reverse }.sort_by(&:size).reverse.flatten.uniq
  end

  # How many cards vanish if we collapse the cards to single values
  def collapsed_size
    size - uniq.size
  end

end

    Together with the Card class, this gives you a nice DSL you can build on:

    hand = Hand.new Card.new(:spades, 14), Card.new(:diamonds, 14), Card.new(:hearts, 14), Card.new(:clubs, 14), Card.new(:clubs, 14)
hand.all? &:ace? #=> true, this guy is obviously cheating

hand.any? &:spades? #=> true, he has spades

hand.count &:ace? #=> 5

    Just for fun, if you would have a Deck class which would also subclass Array

    class Deck < Array
  # the hands this deck creates
  attr_reader :hands

  # You can install any order here, Bridge, Preferans, Five Hundred
  SUITS = %i(clubs diamonds hearts spades).freeze

  # Initialize a deck of cards
  def initialize
    super (Card.new(SUITS.first, 1)..Card.new(SUITS.last, 14)).to_a
    shuffle!
  end

  # Deal n hands
  def deal! hands=5
    @hands = hands.times.map {|i| Hand.new *pop(5) }
  end

  # ... and so on
end

deck = Deck.new
deck.deal!

deck.hands.sort_by &:rank #see who's winning

hand = deck.hands.first

# Select cards left in the deck that could be helpful to this hand
deck.select do |card|
  hand.any?{|card_in_hand| (card_in_hand..card_in_hand.succ).include? card }
end

  2. Where to go further

    • Implement Comparable on Hand
    • Get rid of freeze on Hand so we can exchange cards for some type of games
    • yada yada yada

As I said, your example is already great and I hope you appreciate me sharing my ideas with you!

Leave a Reply

Your email address will not be published. Required fields are marked *