patternjavaMinor
Poker game classes
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
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,
And this avoids it a little more,
Is n't this off by 1?
And here, condition is bad
Also note that your spelling of strength varies.
A little more drying up here
Now, these are probably better off in a case structure
That is,
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.