-
Notifications
You must be signed in to change notification settings - Fork 2
Allie's poker implementation #1
base: master
Are you sure you want to change the base?
Conversation
Also added return in specs for cyo_cards since without it caused an issue with the case statement
This refactors the one pair spec to remove a random failure caused by the generated card set occasionally having a pair (approx. one of out every 13 tries) and removes the reliance on the `cyo_cards` method. We're going to do this refactoring on some of the other tests soon.
Refactor one pair spec
We discovered a lacking context within our full house which has been amended to make sure that three of a kind will not be considered a full house on its own.
Full house implementation with testing
…, move it inside of create method
…a tie, added specs
require 'poker/deck' | ||
require 'poker/hand' | ||
require 'poker/dealer' | ||
# frozen_string_literal: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# frozen_string_literal
should be the top level comment.
end | ||
|
||
def deal_round | ||
@num_players.times do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hands = Array.new(@num_players) { deal }
|
||
def initialize | ||
@cards = [] | ||
return create! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initialize
methods don't return anything.
@cards << Poker::Card.new(rank, suit) | ||
end | ||
end | ||
@cards.shuffle! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cards = Card::RANKS.flat_map do |rank|
Suit::SUITS.each.flat_map do |suit|
Card.new(rank, suit)
end
end.shuffle
@cards = cards | ||
@sets = sets | ||
|
||
raise "Hey! I don't have enough cards" if @cards.length != 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can create a custom error for this quite easily.
InvalidHandError = Class.new(StandardError)
expect(deck.cards).to be_kind_of(Array) | ||
end | ||
it 'receives create on initialize' do | ||
expect_any_instance_of(Poker::Deck).to receive(:create!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stubbing methods on the class under test isn't generally good. Why is this important?
Poker::Card.new('4', 'Diamonds'), | ||
] | ||
|
||
expect { Poker::Hand.new(cards) }.to raise_error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should raise a specific error. This can end up matching lots of things.
@hands.each {|h| hand_rankings[h.level] = [] } | ||
@hands.each do |h| | ||
hand_rankings[h.level] << h | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method itself is a bit complicated. Lets talk about some alternatives.
|
||
def initialize(cards) | ||
@cards = cards | ||
@sets = sets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method in which this is memoized is a bit weird. We should contain this logic in the method below.
@cards.each do |c| | ||
sets[c.number_value] << c | ||
end | ||
sets.sort_by {|k,v| k}.to_h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we get away with something like:
@cards.inject(Hash.new(0)) do |hash, card|
hash[card.number_value] += 1
hash
end
… sure to match all contents of array
No description provided.