tags:

views:

284

answers:

10

I am having problems with my full house method. I thought it was as simple as checking for three of a kind and a pair. But with my current code i am getting a full house with only a three of a kind. Code for isFullHouse() isThreeOfAKind() and isPair() is below thanks for all the help!

 public boolean isPair() {
     Pips[] values = new Pips[5];
     int count =0;

     //Put each cards numeric value into array
     for(int i = 0; i < cards.length; i++){
         values[i] = cards[i].getPip();
     }

     //Loop through the values. Compare each value to all values
     //If exactly two matches are made - return true
     for(int x = 1; x < values.length; x++){
         for(int y = 0; y < x; y++){
             if(values[x].equals(values[y])) count++;
         }
         if (count == 1) return true;
         count = 0;
     }
     return false;  
 }

 public boolean isThreeOfAKind() {
    Pips[] values = new Pips[5];
    int counter = 0;

    for(int i = 0; i < cards.length; i++){
        values[i] = cards[i].getPip();
    }

    //Same process as isPair(), except return true for 3 matches
    for(int x = 2; x < values.length; x++){
         for(int y = 0; y < x; y++){
             if(values[x].equals(values[y]))
                 counter++;
         }
         if(counter == 2) return true;
         counter = 0;
    }

    return false;
}

public boolean isFullHouse(){
    if(isThreeOfAKind() && isPair())
        return true;
    return false;
}
+6  A: 

Check to make sure that the pair is of a different rank than the three of a kind. Otherwise, your isPair() function will find the same cards as the three of a kind. Maybe like this:

public boolean isFullHouse(){
    int three = isThreeOfAKind();
    int pair = isPair();
    if (three != 0 && pair != 0 && three != pair) {
        return true;
    }
    return false;
}

(I used int, but you could change to use your Pips type if you like.)

Greg Hewgill
There's nothing there that would stop if from returning the same pair as isThreeOfAKind. It will give you false negatives.
Paul Tomblin
@Paul Tomblin: If the implementation of `isPair()` *only* returns pairs and not pairs from a triple, then that would work.
Greg Hewgill
If you're going to assume that other methods are not going to work the way they do in the original code, then you should state so in your answer.
Paul Tomblin
Greg, I understand what you are saying but then that would mean i would have to change my methods from boolean to return an integer value. This would mean i wouldn't be able to use them to check only for a pair or three of a kind. right?
LKANL
@LKANL: Yes, you would have to change your code. You could still use them to check for the other kinds by themselves, like `isPair() != 0` and `isThreeOfAKind() != 0` (in those cases, it doesn't matter what the rank of the cards are). Also once you need to compare two hands to see who wins, you might need to know what rank the pair/three are anyway.
Greg Hewgill
Ok i understand, Thanks for the help!
LKANL
+1  A: 

because three of a kind has a pair as well (actually would probably be 2 pairs in your code)

one way to do this is to sort the hand by rank, then its just conditionals to detect a boat.

if ( ((c1.rank == c2.rank == c3.rank) && (c4.rank == c5.rank)) ||
     (c1.rank == c2.rank) && (c3.rank == c4.rank == c5.rank))

ther emight be an extra ( in there but you get the idea...

hvgotcodes
... or possibly three pairs (AB, AC, BC)
Bob Kaufman
KevinDTimm
@kevindtimm absolutely correct -- thats why this type of thing is ripe for unit testing...
hvgotcodes
A: 

You need to make sure the pair is a different two cards than the three of a kind. If the hand is A A A 7 8, then both ThreeOfAKind and isPair return true because you have three aces (and a pair of aces).

Adrian McCarthy
+2  A: 

You have to remove the three of a kind cards from the five card hand first. Three of a kind is true implies two of a kind is true. The sets need to be disjoint.

dougvk
A: 

Hi, Your isPair() method will always return true when there are three cards of a kind because your inner loop always tests the y values only up to x.

so with this data AAA78, when x = 1 y = 0 you will get count == 1 in the inner loop and return true although there are three of a kind. It's better to loop over the entire array and count values when

if(values[x].equals(values[y]) && x != y)

Besides - it's better to use one function in the form of isNOfAKind() which gets the amount of cards as a parameter since these two methods essentially do the same.

Hila
+3  A: 

Can I suggest a way of making your logic dramatically simpler?

Consider a helper method named partitionByRank():

public class RankSet {
    private int count;
    private Rank rank;
}

/**
 * Groups the hand into counts of cards with same rank, sorting first by
 * set size and then rank as secondary criteria
 */
public List<RankSet> partitionByRank() {
   //input e.g.: {Kh, Qs, 4s, Kd, Qs}
   //output e.g.: {[2, K], [2, Q], [1, 4]}
}

Getting the type of hand is really easy then:

public boolean isFullHouse() {
    List<RankSet> sets = partitionByRank();
    return sets.length() == 2 && sets.get(0).count == 3 && sets.get(1).count() == 2;
}

public boolean isTrips() {
    //...
    return sets.length() == 3 && sets.get(0).count = 3;
}

This will also help later on when you inevitably need to check whether one pair is greater than another pair, e.g.

Mark Peters
A: 

Just an idea, wouldn't it be easier to do something like this:

int[] count=new int[13];//size of all ranks
for (i=0;i<5;i++)
  count[ card[i].rank ] ++;

So you will have for example: 0 0 0 0 0 3 0 0 0 2 0 0 0 0 for a full house. A straight would look like 5 ones in a row: 0 0 0 0 1 1 1 1 1 0 0 0.

Since the methods are public, I would not like the isPair() method to return true if there is a pair. It should only return true if there is nothing better than one pair.

Ishtar
+1  A: 

You are missing a third condition: the triple needs to be different cards than the pair. Soo... since you have this shared "cards" array, you probably could "mark" the cards as counted, and reset the counted status for each pass:

//Same process as isPair(), except return true for 3 matches
for(int x = 2; x < values.length; x++){
     cards[x].setCounted(true);  // by default, count the start card
     for(int y = 0; y < x; y++){
         // make sure the card isn't already counted:
         if(!cards[y].isCounted() && values[x].equals(values[y])) {
             counter++;
             cards[x].setCounted(true); // count it
         }
     }
     if(counter == 2) return true;
     counter = 0;
     // reset counted cards
     for(int z=0, zlen=values.length; z < zlen; z++) { cards[z].setCounted(false); }
}
Jeff Meatball Yang
A: 

A better general approach to the problem - this is C#, but converting it to Java should be straightforward:

int[] countOfRank = new int[13];
int[] countOfSuit = new int[4];
for(int i = 0; i < cards.length; i++)
{
     countOfRank[cards[i].Rank]++;
     countOfSuit[cards[i].Suit]++;
}

for (int i=0; i < countOfSuit.length; i++)
{
   isFlush = isFlush || countOfSuit[i] == 5;
}

int[] countOfTuple = new int[5];
int runLength=0;
for (int i=0; i < countOfRank.length; i++)
{
   if (countOfRank[i] == 1)
   {
      runLength++;
      isStraight = (isStraight || runLength == 5);
   }
   else
   {
      runLength=0;
   }
   countOfTuple[countOfRank[i]]++;
}
isPair = (countOfTuple[2] == 1 && countOfTuple[3] == 0);
isTwoPair = (countOfTuple[2] == 2);
isFullHouse = (countOfTuple[2] == 1 && countOfTuple[3] == 1);
isThreeOfAKind = (countOfTuple[2] == 0 && countOfTuple[3] == 1);
isFourOfAKind = (countOfTuple[4] == 1);
isStraightFlush = (isStraight && isFlush);
isStraight = (isStraight && !isStraightFlush);
isFlush = (isFlush && !isStraightFlush);
isRoyalFlush = (isStraightFlush && countOfRank[12] == 1);
isStraightFlush = (isStraightFlush && !isRoyalFlush);
Robert Rossney
A: 

If you're only dealing with five-card hands, counting the number of pairs should yield one for a pair, two for two-pair, three for three-of-a-kind (e.g. if one has As, Ad, and Ac, the pairs are As-Ad, As-Ac, and Ad-Ac), four for a full house, and six for four-of-a-kind. This logic will not work with seven card hands, since it would count three for e.g. A-A-K-K-Q-Q-J (which should only count as two-pair, not three-of-a-kind), and six for A-A-A-K-K-K-Q (which should count as a full house, not four-of-a-kind).

supercat