2vs2 Poker Hand One Pair Winner Algorithm

Question

I wrote this algorithm to determine the winning hand of two players going head to head with a hand of One Pair. Note that the hand is sorted once it is given out, and the algorithm has to compare the two hands one pair, if one has a higher one pair such as AA vs 33, there is an obvious winner. If the hands have matching one pairs, such as 44 & 44, the high card must be compared to determine the winner.

Please take a look at let me know how it looks. It is my first real algorithm taking into account so many factors. I am looking more for opinions, not answers. If you feel it is no good and needs improvement, please tell me so. It does work 100% as I have ran a lot of tests using different one pair hands of different values.

         // Arrays to hold matching pair and high card
         PlayingCard[] pPairHigh = new PlayingCard[2];
         PlayingCard[] oPairHigh = new PlayingCard[2];
         for(int i = 0; i < POKER_HAND_SIZE - 1 ; i++) {

             if(this.hand[i].compareTo(this.hand[i+1]) == 0) {
                 pPairHigh[0] = this.hand[i];

                 if(i == 3)
                    pPairHigh[1] = this.hand[i - 1];
                 else
                    pPairHigh[1] = this.hand[POKER_HAND_SIZE - 1];
             }

             if(otherHand.hand[i].compareTo(otherHand.hand[i+1]) == 0) {
                 oPairHigh[0] = otherHand.hand[i];

                 if(i == 3)
                    oPairHigh[1] = otherHand.hand[i - 1];
                 else
                    oPairHigh[1] = otherHand.hand[POKER_HAND_SIZE - 

         if(pPairHigh[0].compareTo(oPairHigh[0]) == 0){
             if(pPairHigh[1].compareTo(oPairHigh[1]) < 0)
                     return -1;
             else if(pPairHigh[1].compareTo(oPairHigh[1]) > 0)
                 return 1;
             else
                 return 0;
         }

         if(pPairHigh[0].compareTo(oPairHigh[0]) < 0)
             return -1;
         else
             return 1;


     }

Answer 1

I'm not really any good at algorithms as such so my main suggestion would be to remove duplicate code. That is primarily in the creation of your pPairHigh and oPairHigh blocks of code.

I admit they do share a common loop which in my implementation means this will be occurring twice. If this is an issue in performance it could be broken out so the pairs are created like you have done in a single loop. But I would still call a method to do the actual creation. However I'm working on the assumption performance is not really an issue, here is my suggestions.

// Please not I made a guess around your method name here
private int ComparePairs() {
    PlayingCard[] myHand = getPokerHand(this.hand);
    PlayingCard[] otherPlayersHand = getPokerHand(otherHand);

    return compareMyHandAgainstOthers(myHand, otherPlayersHands)
 }

 private int compareMyHandAgainstOthers(PlayingCard[] myHand, PlayingCard[] otherHand) {     
    if(pPairHigh[0].compareTo(oPairHigh[0]) == 0) {
        if(pPairHigh[1].compareTo(oPairHigh[1]) < 0)
           return -1;
        else if(pPairHigh[1].compareTo(oPairHigh[1]) > 0)
           return 1;
        else
           return 0;
    }

    if(pPairHigh[0].compareTo(oPairHigh[0]) < 0)
       return -1;
    else
       return 1;
 }   

 private PlayingCard[] getPokerHand(Array hand) {

   PlayingCard[] pair = new PlayingCard[2];
   for(int i = 0; i < POKER_HAND_SIZE - 1 ; i++) {
        if(this.hand[i].compareTo(this.hand[i+1]) == 0) {
            pair[0] = this.hand[i];

            if(i == 3)
               pair[1] = this.hand[i - 1];
            else
               pair[1] = this.hand[POKER_HAND_SIZE - 1];

        }
    }

    // throw exception here as no possible pair found???
     return pair;
 }

Please note there may be java specific code as I haven't programmed in Java for a while.