Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rank hands without tiebreaker #2

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions hand.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
require "./card"
require "./hand_ranker"

class Hand
def initialize(cards)
@cards = cards
@rank = HandRanker.new(self).rank
end

attr_accessor :rank
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these attr_accessors? That exposes a lot of internal state to other classes. How about just doing attr_reader instead?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, all attr_* methods should be together: attr_reader :rank, :cards.

attr_accessor :cards

def to_s
@cards.join("\n")
end

def <=>(other_hand)
rank <=> other_hand.rank
end

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots of extra lines



end
89 changes: 89 additions & 0 deletions hand_classifier.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
class HandClassifier
def initialize(hand)
@hand = hand
@rank = classify
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is doing a lot of work in an initializer. How about not having @rank at all and forcing other classes to call .classify?

end

attr_reader :hand
attr_accessor :rank
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What other class is setting the rank attribute?


def classify
if straight_flush?
:straight_flush
elsif four_of_a_kind?
:four_of_a_kind
elsif full_house?
:full_house
elsif flush?
:flush
elsif straight?
:straight
elsif three_of_a_kind?
:three_of_a_kind
elsif two_pair?
:two_pair
elsif one_pair?
:one_pair
else
:high_card
end
end

def straight_flush?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything here and below should be private, I think.

straight? && flush?
end

def four_of_a_kind?
find_pairings?(4)
end

def full_house?
grouped_hand = find_matches
full_house = grouped_hand.length == 2 && grouped_hand.any? do |group|
group.length == 2
end
full_house && grouped_hand.any?{ |group| group.length == 3 }

end

def flush?
hand.cards.map(&:suit).reduce(true, :==) #i apologize for the accidental obscenity of this code
end

def straight?
values = hand.cards.map(&:value)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More than 5 lines 😠

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was definitely the method i had the most trouble with. I feel like there should be some way to use reduce here but I can't figure it out...

values.sort!
increment = values[0]
match_tracker = true
values.each_with_index do |value, index|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try looking into the any? method on Enumerables.

match_tracker = match_tracker && index + increment == value
end
match_tracker
end

def three_of_a_kind?
find_pairings?(3)
end

def two_pair?
matches = find_pairings(2)
matches.length == 2
end

def one_pair?
find_pairings?(2)
end

def find_matches
hand.cards.group_by { |card| card.value }
end

def find_pairings?(length)
!find_matches.select { |number, group| group.length == length }.empty?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be: find_matches.any? { |number, group| group.length == length }

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you can use find_pairings(length).empty?, which is even better.

end

def find_pairings(length)
find_matches.select { |number, group| group.length == length }
end
end

37 changes: 37 additions & 0 deletions hand_ranker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
require "./hand_classifier"
class HandRanker
HAND_RANKS = {
:straight_flush => 9,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use fancy symbol-key style: straight_flush: 9

:four_of_a_kind => 8,
:full_house => 7,
:flush => 6,
:straight => 5,
:three_of_a_kind => 4,
:two_pair => 3,
:one_pair => 2,
:high_card => 1
}

def initialize(hand)
@hand = hand
@rank = rank_hand
end

attr_accessor :hand, :rank

def <=>(other_hand)
if rank > other_hand.rank || rank < other_hand.rank
rank <=> other_hand.rank
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just do this always? It will Do The Right Thing in all cases. I think the if is unnecessary.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my big problem is if rank is equal and i need to do tiebreaking in the future

else
# TieBreaker.new(hand, other_hand)
puts "tiebreaker functionality does not exist yet. All players have been exterminated"
0
end
end

private

def rank_hand
HAND_RANKS[HandClassifier.new(hand).rank]
end
end
5 changes: 4 additions & 1 deletion poker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@ def deal_hands
end

def display_hands
puts "Hands ranked highest to lowest"
@hands.sort!.reverse!
@hands.each_with_index do |hand, index|
puts "Player #{index + 1}:"
puts hand
puts hand.rank
puts
end
end
end

poker = Poker.new(4)
poker = Poker.new(6)
poker.play