HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Poker game classes

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pokergameclasses

Problem

I've built this two classes for a Poker game on Android and I would appreciate some feedback on my code.

Have no mercy. The harsher you are, the better.

Feel free to add your number of WTFs/minute.

PokerHand.java

```
package com.poker.util;

import java.util.Arrays;
import java.util.Collections;

public class PokerHand implements Comparable {
public static final int NUM_CARDS = 5;

public enum Strength {
HIGH_CARD, ONE_PAIR, TWO_PAIR, THREE_OF_A_KIND, STRAIGHT, FLUSH, FULL_HOUSE, FOUR_OF_A_KIND, STRAIGHT_FLUSH
}

/**
* array of cards sorted descending
*/
public Card[] cards;

protected Strength strength = null;

/**
* int strenghtValue has 4 bits per relevant card
*
* It is used to differentiate between two hands of the same strength
*
* For example if the hand strength == TWO_PAIR
* c0 will be the top pair and c1 will be the bottom pair
*
*
* strenghtValue
* 0000
* 0000
* 0000
* 0000
* 0000
* 0000
* 0000
* 0000
*
*
* index
*
*
*
* c0
* c1
* c2
* c3
* c4
*
*
*
* Where c0 is the most important rank
*/
private int strengthValue = 0;
private static final int[] MASK_CARD = { 0x000F0000, 0x0000F000,
0x00000F00, 0x000000F0, 0x0000000F };

public PokerHand(Card c1, Card c2, Card c3, Card c4, Card c5) {
cards = new Card[NUM_CARDS];
this.cards[0] = c1;
this.cards[1] = c2;
this.cards[2] = c3;
this.cards[3] = c4;
this.cards[4] = c5;
Arrays.sort(cards,Collections.reverseOrder());
evaluateSrenght();
}

public PokerHand(int c1, int c2, int c3, int c4, int c5) {
cards = new Card[NUM_CARDS];
this.cards[0] = new Card(c1);
this.cards[1] = new Card(c2);
this.cards[2] = new Card(c3);
this.cards[3] = new Card(c4);
this.c

Solution

You could try drying it a little bit,

public init(Card c1, Card c2, Card c3, Card c4, Card c5) {
    cards = new Card[NUM_CARDS];
    this.cards[0] = c1;
    this.cards[1] = c2;
    this.cards[2] = c3;
    this.cards[3] = c4;
    this.cards[4] = c5;
    Arrays.sort(cards,Collections.reverseOrder());
    evaluateSrenght();
}

public PokerHand(Card c1, Card c2, Card c3, Card c4, Card c5) {
    init(c1, c2, c3, c4, c5);
}
public PokerHand(int c1, int c2, int c3, int c4, int c5) {
    init(new Card(c1), new Card(c2), new Card(c3), new Card(c4), new Card(c5));
}


And this avoids it a little more,

public int compareTo(PokerHand hand) {
    int i = this.getStrength().compareTo(hand.getStrength());
    if (i != 0)
       return i;
    return (new Integer(strengthValue)).compareTo(new Integer(hand.strengthValue));
}


Is n't this off by 1?

private int getNumPairs() {
    int pairs = 0;
    for (int i = 0; i < NUM_CARDS; i++)
        if (cards[i].rank == cards[i + 1].rank)
            pairs++;

    return pairs;
}


And here, condition is bad

private void evaluateSrenght() {
...
    else{
        strength = Strength.HIGH_CARD;
        for (int i=0; i>NUM_CARDS; i++)
            setSrengthValue(i, cards[i].rank);
    }
}


Also note that your spelling of strength varies.

A little more drying up here

private boolean cmpAllranks(int[] c1, int[] c2) {
   for(int i = 0; i < c1.length; i++)
         if (cards[c1[i]].rank != cards[c2[i]].rank) 
            return false;
}
private boolean isFullHouse( ) {
    if (cards[1].rank == cards[2].rank) {
        return cmpAllranks(new Integer[] {0,1,3}, new Integer[] {1,2,4});
    } else {
        return cmpAllranks(new Integer[] {0,2,3}, new Integer[] {1,3,4});
    }
}
// Choose the style you like
private boolean isFourOfAKind( ) {
    Integer[] c1, c2;
    if (cards[0].rank != cards[1].rank) {
        c1 = new Integer[] {1,2,3};
        c2 = new Integer[] {2,3,4}
    } else {
        c1 = new Integer[] {0,1,2};
        c2 = new Integer[] {1,2,3}
    }
    return cmpAllranks(c1, c2);
}


Now, these are probably better off in a case structure

private void evaluateSrenght() {
    ...
    if (isStraightFlush()) {
    else if (isFourOfAKind()) {
    else if (isFullHouse()) {
    else if (isFlush()){
    else if (isStraight()){
    else if (isThreeOfAKind()){
    else if ( ( numPairs = getNumPairs()) == 2 ) {
    else if (numPairs == 1){
    else{
}


That is,

private void evaluateSrenght() {
    ...
    swtich (typeOfHand()) {
      straightFlush:...
      fullHouse: ...
      flush: ...
      etc.
    }
}

Code Snippets

public init(Card c1, Card c2, Card c3, Card c4, Card c5) {
    cards = new Card[NUM_CARDS];
    this.cards[0] = c1;
    this.cards[1] = c2;
    this.cards[2] = c3;
    this.cards[3] = c4;
    this.cards[4] = c5;
    Arrays.sort(cards,Collections.reverseOrder());
    evaluateSrenght();
}


public PokerHand(Card c1, Card c2, Card c3, Card c4, Card c5) {
    init(c1, c2, c3, c4, c5);
}
public PokerHand(int c1, int c2, int c3, int c4, int c5) {
    init(new Card(c1), new Card(c2), new Card(c3), new Card(c4), new Card(c5));
}
public int compareTo(PokerHand hand) {
    int i = this.getStrength().compareTo(hand.getStrength());
    if (i != 0)
       return i;
    return (new Integer(strengthValue)).compareTo(new Integer(hand.strengthValue));
}
private int getNumPairs() {
    int pairs = 0;
    for (int i = 0; i < NUM_CARDS; i++)
        if (cards[i].rank == cards[i + 1].rank)
            pairs++;

    return pairs;
}
private void evaluateSrenght() {
...
    else{
        strength = Strength.HIGH_CARD;
        for (int i=0; i>NUM_CARDS; i++)
            setSrengthValue(i, cards[i].rank);
    }
}
private boolean cmpAllranks(int[] c1, int[] c2) {
   for(int i = 0; i < c1.length; i++)
         if (cards[c1[i]].rank != cards[c2[i]].rank) 
            return false;
}
private boolean isFullHouse( ) {
    if (cards[1].rank == cards[2].rank) {
        return cmpAllranks(new Integer[] {0,1,3}, new Integer[] {1,2,4});
    } else {
        return cmpAllranks(new Integer[] {0,2,3}, new Integer[] {1,3,4});
    }
}
// Choose the style you like
private boolean isFourOfAKind( ) {
    Integer[] c1, c2;
    if (cards[0].rank != cards[1].rank) {
        c1 = new Integer[] {1,2,3};
        c2 = new Integer[] {2,3,4}
    } else {
        c1 = new Integer[] {0,1,2};
        c2 = new Integer[] {1,2,3}
    }
    return cmpAllranks(c1, c2);
}

Context

StackExchange Code Review Q#10973, answer score: 6

Revisions (0)

No revisions yet.