Why does my isFullHouse() method also accept a simple three-of-a-kind?

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 pro开发者_StackOverflow社区cess 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;
}

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.)

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.

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.

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); }
}

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...

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).

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.

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.

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);

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).

According to your code inlined comments (exactly two matches words) maybe you are trying to implement isPair method in such a way that it will return false in case of three of a kind combination. If so, you need change your isPair method to iterate over all items in the array, like this:

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