Skip to main content
Code Review

Return to Answer

added 75 characters in body
Source Link
Simon Forsberg
Simon Forsberg
  • 59.8k
  • 9
  • 160
  • 312

Oh, a wildcard! But this is still considered a three of a kind, right? Wrong. This is consideredIt should be a three of a kind, but the code considers this a full house because ace counts forcan be both 14high (14) and 1low (1), so we have a full house with three 14's and two 1's.

This isThe code still consideredconsiders this a full house of both high and low aces.

That's got to be one pair right? Nope, the code says that wasis two pair. Again, two pair of both high and low aces.

So what is thea possible fix for this? In checkForFullHouseAndStuff in PokerPair, pass the results first through the following method:

Oh, a wildcard! But this is still considered a three of a kind, right? Wrong. This is considered a full house because ace counts for both 14 and 1, so we have a full house with three 14's and two 1's.

This is still considered a full house of both high and low aces.

That's got to be one pair right? Nope, that was two pair. Again, two pair of both high and low aces.

So what is the fix for this? In checkForFullHouseAndStuff in PokerPair, pass the results first through the following method:

Oh, a wildcard! But this is still considered a three of a kind, right? Wrong. It should be a three of a kind, but the code considers this a full house because ace can be both high (14) and low (1), so we have a full house with three 14's and two 1's.

The code still considers this a full house of both high and low aces.

That's got to be one pair right? Nope, the code says that is two pair. Again, two pair of both high and low aces.

So what is a possible fix for this? In checkForFullHouseAndStuff in PokerPair, pass the results first through the following method:

Source Link
Simon Forsberg
Simon Forsberg
  • 59.8k
  • 9
  • 160
  • 312

Almost four years later... I discovered a bug. Consider the following tests:

assertEquals(PokerHandType.THREE_OF_A_KIND, eval.evaluate( card(Suite.CLUBS, ClassicCard.RANK_ACE_HIGH), card(Suite.HEARTS, ClassicCard.RANK_5), card(Suite.SPADES, ClassicCard.RANK_KING), card(Suite.DIAMONDS, ClassicCard.RANK_ACE_HIGH), card(Suite.SPADES, ClassicCard.RANK_ACE_HIGH) ).getType());

That's a three of a kind and should pass, right? Actually yes, it does.

Next one:

assertEquals(PokerHandType.THREE_OF_A_KIND, eval.evaluate( card(Suite.CLUBS, ClassicCard.RANK_ACE_HIGH), card(Suite.SPADES, ClassicCard.RANK_ACE_HIGH), card(Suite.SPADES, ClassicCard.RANK_KING), card(Suite.HEARTS, ClassicCard.RANK_8), card(Suite.HEARTS, ClassicCard.RANK_5), card(Suite.SPADES, ClassicCard.RANK_2), card(Suite.EXTRA, ClassicCard.RANK_WILDCARD) ).getType());

Oh, a wildcard! But this is still considered a three of a kind, right? Wrong. This is considered a full house because ace counts for both 14 and 1, so we have a full house with three 14's and two 1's.

We can even get rid of a few extra cards and take a look at this one:

assertEquals(PokerHandType.THREE_OF_A_KIND, eval.evaluate( card(Suite.CLUBS, ClassicCard.RANK_ACE_HIGH), card(Suite.SPADES, ClassicCard.RANK_ACE_HIGH), card(Suite.EXTRA, ClassicCard.RANK_WILDCARD) ).getType());

This is still considered a full house of both high and low aces.

How about something simpler then?

assertEquals(PokerHandType.PAIR, eval.evaluate( card(Suite.CLUBS, ClassicCard.RANK_ACE_HIGH), card(Suite.DIAMONDS, ClassicCard.RANK_ACE_HIGH) ).getType());

That's got to be one pair right? Nope, that was two pair. Again, two pair of both high and low aces.

So what is the fix for this? In checkForFullHouseAndStuff in PokerPair, pass the results first through the following method:

private PokerHandResult fixDoubleAce(PokerHandResult result, PokerHandAnalyze analyze) { if (result.getPrimaryRank() != ClassicCard.RANK_ACE_HIGH || result.getSecondaryRank() != ClassicCard.RANK_ACE_LOW) { return result; } switch (result.getType()) { case FULL_HOUSE: return new PokerHandResult(PokerHandType.THREE_OF_A_KIND, result.getPrimaryRank(), 0, analyze.getCards()); case TWO_PAIR: return new PokerHandResult(PokerHandType.PAIR, result.getPrimaryRank(), 0, analyze.getCards()); default: throw new IllegalStateException("Unexpected scenario cleaning double ace from " + result); } }

This method first checks if the result is for both high and low aces and if it is, it returns a degraded poker hand of only the primary rank. So full house becomes three of a kind and two pair becomes one pair.