Random not so random right now....
I am building a texas holdem poker game in java. I have made a Card class like this:public Card(int val,int s)
where val is a number between 0 and 13 representing the card number and s is an integer between 0 and 4 representing the suit.
Next I built a Deck class that populates a Card[] with 52 random cards, but as it does this it checks to see if the random card has been put in the deck already, and if it has then it gets a new random card. The problem is that it seems to be filling the deck with only like 3 - 5 different cards. I cant figure out how it wud choose the same card so many times. Here is the Deck class:
package poker;
import java.util.Random;
import java.util.ArrayList;
publicclass Deck{
public Card[] doc;
publicint nextCard;
public ArrayList deckList;
public Deck(){
deckList =new ArrayList();
doc = populateDeck();// fills the Card[] with 52 "random" Card objects
doc = checkDeck();// a double checking method that really shudnt exist
printDeck(doc);// prints each card to screen
nextCard = 0;
}
public Card[] populateDeck(){
Card current;
Card[] result;
for (int i = 0; i < 52; i++){
current = getRandomCard();// gets a card of random value and suit
while (!isFreshCard(current)){// if it isnt an unseen card, get a new card until it finds one
current = getRandomCard();
}
deckList.add(current);
}
deckList.trimToSize();
return objToCards(deckList.toArray());// returns a Card[52] array
}
public Card getRandomCard(){
Random rnd =new Random();
int val = rnd.nextInt(13);// gets an int between 0 and 13 for the value
int suit = rnd.nextInt(4);// gets an int between 0 and 4 for the suit
returnnew Card(val, suit); constructs and returns annew card with these random ints
}
publicboolean isFreshCard(Card current){// checks to see if the card parameter is in the deck arraylist already
return (!deckList.contains(current));
}
public Card[] checkDeck(){// shud be useless, may as well ignore
ArrayList usedDeck =new ArrayList();
Card current;
for (int i = 0; i < 52; i++){
usedDeck.add(doc[i]);
}
for (int j = 0; j < 52; j++){
if (usedDeck.indexOf(doc[j]) != usedDeck.lastIndexOf(doc[j])){
current = getRandomCard();
while (!isFreshCard(current)){
current = getRandomCard();
}
usedDeck.set(j, current);
}
}
return objToCards(usedDeck.toArray());
}
publicstatic Card[] objToCards(Object[] array){// Object[] to Card[]
Card[] carray =new Card[array.length];
for (int i = 0 ; i < array.length ; i++)
carray[i] = (Card) array[i];
return carray;
}
publicvoid printDeck(Card[] deck){//prints each card to the terminal
for (int i = 0; i < deck.length; i++){
System.out.println(deck[i].toString());
}
System.out.println();
System.out.println(deck.length +" cards.");
}
// the rest shud work fine and therefore can be ignored.
public Card getNextCard(){
Card next;
next = doc[nextCard];
nextCard++;
System.out.println(next.toString());
return next;
}
public Card[] getNextCards(int cards){
Card[] result =new Card[cards];
for (int i = 0; i < cards; i++){
result[i] = getNextCard();
}
return result;
}
publicvoid burn(){nextCard++;}
}
any ideas why i cant get 52 random unique cards? or any better ways to do this?
[7052 byte] By [
llampwalla] at [2007-11-27 1:56:23]
Hi, Set a seed to your Random instance see Random.setSeed.Hope that help, Jack
where val is a number between 0 and 13 representing the card number and s is an integer between 0 and 4 representing the suitI think you mean between 0 and 12 and 0 and 3But anyway, create your instance of random as an instance method of the Deck class.
> But anyway, create your instance of random as an> instance method of the Deck class.... and what does that mean exactly?
It means that you should move this line:
Random rnd = new Random();
out of the getRandomCard() method.
If you want to know more about it, look for things on random number seeding. In fact, this was linked in another thread today, interesting read:
http://mindprod.com/jgloss/pseudorandom.html#PSEDORANDOM
You may also be interested to hear that a few years ago, an online poker site made a similar mistake with a pseudo random number generator. It is common practice for these sites to publish their "shuffle" alogrithm. Hilarity ensued.
Instead of declaring it inside the method, declare at the top of the class.
> You may also be interested to hear that a few years
> ago, an online poker site made a similar mistake with
> a pseudo random number generator. It is common
> practice for these sites to publish their "shuffle"
> alogrithm. Hilarity ensued.
Wow! They must have gone through a very thorough and exhaustive testing phase.
> > You may also be interested to hear that a few
> years
> > ago, an online poker site made a similar mistake
> with
> > a pseudo random number generator. It is common
> > practice for these sites to publish their
> "shuffle"
> > alogrithm. Hilarity ensued.
>
> Wow! They must have gone through a very thorough and
> exhaustive testing phase.
Clearly. ;-)
I don't remember the exact details, it wasn't as simple as just not seeding the random number correctly, but it made it easy(ier) to predict a pattern in the shuffle algorithm with some margin of error.
I guess using Collections.shuffle() on an ArrayList of every possible card would be the best way to handle this. Fixing my actual Random generator wasnt really wat i was concerned with, becuz even if it wasnt working, my isFreshCard() method should not let it repeat a card, but watever ill try this way.
If all you were trying to do was create a deck of 52 cards then generating the cards yourself and adding them to the List and then shuffling is the best way to go. You will need two nested for loops. One goes around 4 times (for suits) and one goes 13 times (for values).
I would just go once around 52 random values and compute suit = value/13, face = value/4. I suspect it may be slightly 'more random' and certainly simpler to code.
ejpa at 2007-7-12 1:30:41 >
Here is my implimentation of OPs problem. Comments appreciated.
package poker;
import java.util.Random;
public class Card implements Comparable {
private Random rand = new Random();
private static final int SPADE = 0;
private static final int CLUB = 1;
private static final int DIAMOND = 2;
private static final int HEART = 3;
private int value;
private int suit;
public Card(int v, int s) {
this.value = v;
this.suit = s;
}
public Card() {
this.value = rand.nextInt(12);
this.suit = rand.nextInt(3);
}
public int getSuit() {
return suit;
}
public void setSuit(int suit) {
this.suit = suit;
}
public int getValue() {
return value;
}
public void setValue(int value) {
this.value = value;
}
public int compareTo(Object o) {
Card input = (Card)o;
if (this.value < input.value) return -1;
else if (this.value > input.value) return 1;
else return 0;
}
public String toString() {
switch(suit) {
case Card.HEART : { return getValue() + ", heart"; }
case Card.SPADE : { return getValue() + ", spade"; }
case Card.DIAMOND : { return getValue() + ", diamond"; }
case Card.CLUB : { return getValue() + ", club"; }
default : return "";
}
}
}
package poker;
import java.util.*;
public class Deck {
public final ArrayList<Card> deck = new ArrayList<Card>();
public Deck() {
populateDeck();
}
private void populateDeck() {
for (int i = 0; i < 4; i++) {
for (int j = 0; j < 13; j++) {
Card newCard = new Card(j, i);
deck.add(newCard);
}
}
}
public void orderDeck() {
Collections.sort(deck);
}
public void shuffleDeck() {
Collections.shuffle(deck);
}
}
package poker;
public class Main {
public static void main(String[] args) {
Deck deck = new Deck();
deck.shuffleDeck();
for (Card c : deck.deck) {
System.out.println(c);
}
}
}
this.value = rand.nextInt(12);
this.suit = rand.nextInt(3);
These upper bounds should be 4 and 13 respectively, as the upper bound is excluded in Random.nextInt().
Card.setSuit() and Card.setValue() should be removed - these attributes should be read-only.
Card.compareTo() should be removed altogether from here, and the Comparable interface. Instead, a variety of Comparator implementations, with and without suit, is needed, depending on what game is being played and indeed in poker what kind of a hand is being constructed.
ejpa at 2007-7-12 1:30:41 >
Thanks.
I have "tightened" up my deck class so that the "deck" is an ArrayList<Card>. It generates 52 Card objects and adds them to the (private instance variable) list in order.
I have left Card as implementing Comparable such that I can now Collections.shuffle() my Deck object.
I have also tightened up the Card class to remove the setters. Very good point.
Does this look better?
package poker;
public class Card implements Comparable<Card>{
private static final int SPADE = 0;
private static final int CLUB = 1;
private static final int DIAMOND = 2;
private static final int HEART = 3;
private int value;
private int suit;
public Card(int v, int s) {
this.value = v;
this.suit = s;
}
public int getSuit() {
return suit;
}
public int getValue() {
return value;
}
public int compareTo(Card input) {
if (this.value < input.value) return -1;
else if (this.value > input.value) return 1;
else return 0;
}
public String toString() {
switch(suit) {
case Card.HEART : { return getValue() + ", heart"; }
case Card.SPADE : { return getValue() + ", spade"; }
case Card.DIAMOND : { return getValue() + ", diamond"; }
case Card.CLUB : { return getValue() + ", club"; }
default : return "";
}
}
}
package poker;
import java.util.*;
public class Deck {
private final int size = 52;
private final List<Card> deck = new ArrayList<Card>();
private int pointer = 0;
public Deck() {
populateDeck();
}
private void populateDeck() {
for (int i = 0; i < 4; i++) {
for (int j = 0; j < 13; j++) {
deck.add(new Card(j, i));
}
}
}
public Card nextCard() throws NullPointerException {
return deck.get(pointer++);
}
public int getSize() {
return size;
}
public void sort() {
Collections.sort(deck);
}
public void shuffle() {
Collections.shuffle(deck);
}
}
> The problem is that it seems to be filling the
> deck with only like 3 - 5 different cards. I cant
> figure out how it wud choose the same card so many
> times.
First are you using a Java version earlier than 1.5?
Second does your Card class override the equals method of Object?
> I have left Card as implementing Comparable such that
> I can now Collections.shuffle() my Deck object.
Collections.shuffle(List<?>) doesn't require Comparable list elements. Collections.sort() does, but again it is game-specific and hand-specific what the sort order is, so sort() should take a Comparator<Card> argument and pass it to Collections.sort().
Deck could implement Iterable<Card>.
ejpa at 2007-7-21 20:15:59 >
> I am building a texas holdem poker game in java. I
> have made a Card class like this:
>public Card(int val, int s)
where val is a number
> between 0 and 13 representing the card number and s
> is an integer between 0 and 4 representing the suit.
...................
> return (!deckList.contains(current));
...................
> any ideas why i cant get 52 random unique cards? or
> any better ways to do this?
The issue is with the call to (!deckList.contains(current));
method.
The List.contains(Object o); method always calls the equals method on the residing objects to see it already contains in the list.
Please see the Java API doc for code]List.contains(Object o);[/code] method :
public boolean contains(Object o)
Returns true if this list contains the specified element. More formally, returns true if and only if this list contains at least one element e such that (o==null ? e==null : o.equals(e)).
So please implement an equals method in the Card class as follows:
public boolean equals(Object o) {
if (o == null || !(o instanceof Card)) {
return false;
}
Card card = (Card)o;
return this.val == card.val && s == card.s;
}
Thanks
> I don't remember the exact details, it wasn't as
> simple as just not seeding the random number
> correctly, but it made it easy(ier) to predict a
> pattern in the shuffle algorithm with some margin of
> error.
The online site was seeding the random number generator with the number of milliseconds past midnight for each deal. The cheaters knew within a few seconds when the deal was started. That meant there were only a few thousand possible seeds that needed to be tested. Since the algorithm was public knowledge, the cheaters were able to run it with the several thousands of possible seeds. When they found the one sequence out of the thousands of test cases that matched their own hand, that gave them the contents of every hand at the table. Rather than make money by using their perfect knowledge of the hidden hands of other poker players, they went public with their knowledge and forced the on-line poker sites to pay more attention to their shuffling algorithms.
