refactored some stuff around, still has a scoring bug

pokerhand.go

@@ -34,121 +34,38 @@ func (c Cards) PokerHand() (*PokerHand, error) {
 	if c.containsDuplicates() {
 		return nil, errors.New("hand must have no duplicates to be scored as a poker hand")
 	}
-	if len(c) < 5 {
-		return nil, errors.New("hand must have at least 5 cards to be scored as a poker hand")
-	}
 
+	ph := new(PokerHand)
+
+	// IdentifyBestFiveCardPokerHand() calls us to score hands
+	// but only if the hand is 5 cards exactly which avoids recursion loop
+	if len(c) > 5 {
 		phc, err := c.IdentifyBestFiveCardPokerHand()
 		if err != nil {
 			// this should in theory never happen
 			return nil, err
 		}
-
-	ph := new(PokerHand)
 		ph.Hand = phc.SortByRankAscending()
-	if ph.Hand.containsRoyalFlush() {
+	} else if len(c) == 5 {
-		ph.Type = RoyalFlush
+		ph.Hand = c.SortByRankAscending()
-		ph.Score = ScoreRoyalFlush
-		return ph, nil
-	}
-
-	if ph.Hand.containsStraightFlush() {
-		ph.Type = StraightFlush
-		ph.Score = ScoreStraightFlush
-		ph.Score += 1000 * ph.Hand.HighestRank().Score()
-		return ph, nil
-	}
-
-	if ph.Hand.containsFourOfAKind() {
-		ph.Type = FourOfAKind
-		ph.Score = ScoreFourOfAKind
-		ph.Score += 1000 * ph.Hand.fourOfAKindRank().Score()
-		ph.Score += 100 * ph.Hand.fourOfAKindKicker().Score()
-		return ph, nil
-	}
-
-	if ph.Hand.containsFullHouse() {
-		ph.Type = FullHouse
-		ph.Score = ScoreFullHouse
-		// FIXME write a good test for this
-		ph.Score += 1000 * ph.Hand.fullHouseTripsRank().Score()
-		ph.Score += 100 * ph.Hand.fullHousePairRank().Score()
-		return ph, nil
-	}
-
-	if ph.Hand.containsFlush() {
-		ph.Type = Flush
-		ph.Score = ScoreFlush
-		// flush base score plus sum of card ranks
-		ph.Score += ph.Hand[0].Score()
-		ph.Score += ph.Hand[1].Score()
-		ph.Score += ph.Hand[2].Score()
-		ph.Score += ph.Hand[3].Score()
-		ph.Score += ph.Hand[4].Score()
-		return ph, nil
-	}
-
-	if ph.Hand.containsStraight() {
-		ph.Type = Straight
-		ph.Score = ScoreStraight
-
-		// Straights are scored by the highest card in the straight
-		// UNLESS the second highest card is a 5 and the highest card is an Ace
-		// In that case, the straight is a 5-high straight, not an Ace-high straight
-		sorted := ph.Hand.SortByRankAscending()
-
-		if sorted[3].Rank == FIVE && sorted[4].Rank == ACE {
-			// 5-high straight, scored by the five's rank
-			ph.Score += sorted[3].Score()
 	} else {
-			// All other straights are scored by the highest card in the straight
+		return nil, errors.New("hand must have at least 5 cards to be scored as a poker hand")
-			ph.Score += sorted[4].Score()
-		}
-		return ph, nil
 	}
 
-	if ph.Hand.containsThreeOfAKind() {
+	// this doesn't return an error because we've already checked for
-		ph.Type = ThreeOfAKind
+	// duplicates and we have already identified the best 5-card hand
-		ph.Score = ScoreThreeOfAKind
-		ph.Score += 1000 * ph.Hand.threeOfAKindTripsRank().Score()
-		ph.Score += 100 * ph.Hand.threeOfAKindFirstKicker().Score()
-		ph.Score += 10 * ph.Hand.threeOfAKindSecondKicker().Score()
-		return ph, nil
-	}
 
-	if ph.Hand.containsTwoPair() {
+	// this code used to be here, but got factored out so it can go in
-		ph.Type = TwoPair
+	// scoring.go
-		ph.Score = ScoreTwoPair
+	ph.calculateScore()
-		ph.Score += 1000 * ph.Hand.twoPairBiggestPair().Score()
-		ph.Score += 100 * ph.Hand.twoPairSmallestPair().Score()
-		ph.Score += 10 * ph.Hand.twoPairKicker().Score()
-		return ph, nil
-	}
 
-	if ph.Hand.containsPair() {
-		ph.Type = Pair
-		ph.Score = ScorePair
-		ph.Score += 1000 * ph.Hand.pairRank().Score()
-		ph.Score += 100 * ph.Hand.pairFirstKicker().Score()
-		ph.Score += 10 * ph.Hand.pairSecondKicker().Score()
-		ph.Score += ph.Hand.pairThirdKicker().Score()
-		return ph, nil
-	}
-
-	ph.Type = HighCard
-	ph.Score = ScoreHighCard // base score
-	// unmade hands are scored like flushes, just add up the values
-	ph.Score += ph.Hand[0].Score()
-	ph.Score += ph.Hand[1].Score()
-	ph.Score += ph.Hand[2].Score()
-	ph.Score += ph.Hand[3].Score()
-	ph.Score += ph.Hand[4].Score()
 	return ph, nil
 }
 
 func (ph PokerHand) ToSortedCards() Cards {
-	sorted := ph.Hand.SortByRankAscending()
+	// I believe ph.Hand is already sorted, but in any case it's only 5
-	return sorted
+	// cards and sorting it costs ~nothing
+	return ph.Hand.SortByRankAscending()
 }
 
 func (ph PokerHand) Compare(other PokerHand) int {
@@ -170,16 +87,15 @@ func (ph PokerHand) String() string {
 }
 
 func (c PokerHand) Description() string {
-	sortedHand := c.Hand.SortByRankAscending()
 	if c.Type == RoyalFlush {
 		return fmt.Sprintf("a royal flush in %s", c.Hand[0].Suit)
 	}
 	if c.Hand.containsStraightFlush() {
-		if sortedHand[3].Rank == FIVE && sortedHand[4].Rank == ACE {
+		if c.Hand[3].Rank == FIVE && c.Hand[4].Rank == ACE {
 			// special case for steel wheel
-			return fmt.Sprintf("%s high straight flush in %s", sortedHand[3].Rank.WithArticle(), sortedHand[4].Suit)
+			return fmt.Sprintf("%s high straight flush in %s", c.Hand[3].Rank.WithArticle(), c.Hand[4].Suit)
 		}
-		return fmt.Sprintf("%s high straight flush in %s", c.HighestRank().WithArticle(), sortedHand[4].Suit)
+		return fmt.Sprintf("%s high straight flush in %s", c.HighestRank().WithArticle(), c.Hand[4].Suit)
 	}
 	if c.Hand.containsFourOfAKind() {
 		return fmt.Sprintf("four %s with %s", c.Hand.fourOfAKindRank().Pluralize(), c.Hand.fourOfAKindKicker().Rank.WithArticle())
@@ -188,12 +104,12 @@ func (c PokerHand) Description() string {
 		return fmt.Sprintf("a full house, %s full of %s", c.Hand.fullHouseTripsRank().Pluralize(), c.Hand.fullHousePairRank().Pluralize())
 	}
 	if c.Hand.containsFlush() {
-		return fmt.Sprintf("%s high flush in %s", c.HighestRank().WithArticle(), sortedHand[4].Suit)
+		return fmt.Sprintf("%s high flush in %s", c.HighestRank().WithArticle(), c.Hand[4].Suit)
 	}
 	if c.Hand.containsStraight() {
-		if sortedHand[3].Rank == FIVE && sortedHand[4].Rank == ACE {
+		if c.Hand[3].Rank == FIVE && c.Hand[4].Rank == ACE {
 			// special case for wheel straight
-			return fmt.Sprintf("%s high straight", sortedHand[3].Rank.WithArticle())
+			return fmt.Sprintf("%s high straight", c.Hand[3].Rank.WithArticle())
 		}
 		return fmt.Sprintf("%s high straight", c.HighestRank().WithArticle())
 	}
@@ -226,10 +142,10 @@ func (c PokerHand) Description() string {
 	return fmt.Sprintf(
 		// "ace high with an eight, a seven, a six, and a deuce"
 		"%s high with %s, %s, %s, and %s",
-		sortedHand[4].Rank,
+		c.Hand[4].Rank,
-		sortedHand[3].Rank.WithArticle(),
+		c.Hand[3].Rank.WithArticle(),
-		sortedHand[2].Rank.WithArticle(),
+		c.Hand[2].Rank.WithArticle(),
-		sortedHand[1].Rank.WithArticle(),
+		c.Hand[1].Rank.WithArticle(),
-		sortedHand[0].Rank.WithArticle(),
+		c.Hand[0].Rank.WithArticle(),
 	)
 }

scoring.go

@@ -34,3 +34,114 @@ func (c Cards) PokerHandScore() (HandScore, error) {
 func (x HandScore) String() string {
 	return fmt.Sprintf("<HandScore %d>", x)
 }
+
+func (ph *PokerHand) calculateScore() {
+
+	// sanity check, we should only be called in the PokerHand() method from
+	// a Cards, but just in case
+	if len(ph.Hand) != 5 {
+		// normally we don't panic in a library but this is a "should never
+		// happen"
+		panic("PokerHand.calculateScore() called on a PokerHand with != 5 cards")
+	}
+
+	if ph.Hand.containsRoyalFlush() {
+		ph.Type = RoyalFlush
+		ph.Score = ScoreRoyalFlush
+		return
+	}
+
+	if ph.Hand.containsStraightFlush() {
+		ph.Type = StraightFlush
+		ph.Score = ScoreStraightFlush
+		ph.Score += 1000 * ph.Hand.HighestRank().Score()
+		return
+	}
+
+	if ph.Hand.containsFourOfAKind() {
+		ph.Type = FourOfAKind
+		ph.Score = ScoreFourOfAKind
+		ph.Score += 1000 * ph.Hand.fourOfAKindRank().Score()
+		ph.Score += 100 * ph.Hand.fourOfAKindKicker().Score()
+		return
+	}
+
+	if ph.Hand.containsFullHouse() {
+		ph.Type = FullHouse
+		ph.Score = ScoreFullHouse
+		// FIXME write a good test for this
+		ph.Score += 1000 * ph.Hand.fullHouseTripsRank().Score()
+		ph.Score += 100 * ph.Hand.fullHousePairRank().Score()
+		return
+	}
+
+	if ph.Hand.containsFlush() {
+		ph.Type = Flush
+		ph.Score = ScoreFlush
+		// flush base score plus sum of card ranks
+		ph.Score += ph.Hand[0].Score()
+		ph.Score += ph.Hand[1].Score()
+		ph.Score += ph.Hand[2].Score()
+		ph.Score += ph.Hand[3].Score()
+		ph.Score += ph.Hand[4].Score()
+		return
+	}
+
+	if ph.Hand.containsStraight() {
+		ph.Type = Straight
+		ph.Score = ScoreStraight
+
+		// note that ph.Hand is already sorted by rank ascending with ace
+		// high
+
+		// Straights are scored by the highest card in the straight
+		// UNLESS the second highest card is a 5 and the highest card is an Ace
+		// In that case, the straight is a 5-high straight, not an Ace-high straight
+		if ph.Hand[3].Rank == FIVE && ph.Hand[4].Rank == ACE {
+			// 5-high straight, scored by the five's rank
+			ph.Score += ph.Hand[3].Score()
+		} else {
+			// All other straights are scored by the highest card in the straight
+			ph.Score += ph.Hand[4].Score()
+		}
+		return
+	}
+
+	if ph.Hand.containsThreeOfAKind() {
+		ph.Type = ThreeOfAKind
+		ph.Score = ScoreThreeOfAKind
+		ph.Score += 1000 * ph.Hand.threeOfAKindTripsRank().Score()
+		ph.Score += 100 * ph.Hand.threeOfAKindFirstKicker().Score()
+		ph.Score += 10 * ph.Hand.threeOfAKindSecondKicker().Score()
+		return
+	}
+
+	if ph.Hand.containsTwoPair() {
+		ph.Type = TwoPair
+		ph.Score = ScoreTwoPair
+		ph.Score += 1000 * ph.Hand.twoPairBiggestPair().Score()
+		ph.Score += 100 * ph.Hand.twoPairSmallestPair().Score()
+		ph.Score += 10 * ph.Hand.twoPairKicker().Score()
+		return
+	}
+
+	if ph.Hand.containsPair() {
+		ph.Type = Pair
+		ph.Score = ScorePair
+		ph.Score += 1000 * ph.Hand.pairRank().Score()
+		ph.Score += 100 * ph.Hand.pairFirstKicker().Score()
+		ph.Score += 10 * ph.Hand.pairSecondKicker().Score()
+		ph.Score += ph.Hand.pairThirdKicker().Score()
+		return
+	}
+
+	ph.Type = HighCard
+	ph.Score = ScoreHighCard // base score
+	// unmade hands are scored like flushes, just add up the values
+	ph.Score += ph.Hand[0].Score()
+	ph.Score += ph.Hand[1].Score()
+	ph.Score += ph.Hand[2].Score()
+	ph.Score += ph.Hand[3].Score()
+	ph.Score += ph.Hand[4].Score()
+	return
+}