My sad little program can't seem to count cards right

[Jeremy Graham]

Here's my Card object:

public class Card {
	String cardName;
	int cardRank;
	int cardSuit;
	
	public void setName(String cn) {
		cardName = cn;
	}

	public String getName() {
		return cardName;
	}

	public void setSuit(int cs) {
		cardSuit = cs;
	}

	public int getSuit() {
		return cardSuit;
	}

	public void setRank(int cr) {
		cardRank = cr;
	}

	public int getRank() {
		return cardRank;
	}
}

Here's my Deck object:

import java.util.*;

public class Deck {
	ArrayList<Card> Deck = new ArrayList<Card>();
	ArrayList<Card> playerHand = new ArrayList<Card>();

	public void setUpDeck() {
		Card SA = new Card();
		SA.setName(" A" + (char)6);
		SA.setSuit(4);
		SA.setRank(14);
		Deck.add(SA);

		Card SK = new Card();
		SK.setName(" K" + (char)6);
		SK.setSuit(4);
		SK.setRank(13);		
		Deck.add(SK);

		Card SQ = new Card();
		SQ.setName(" Q" + (char)6);
		SQ.setSuit(4);
		SQ.setRank(12);
		Deck.add(SQ);

		Card SJ = new Card();
		SJ.setName(" J" + (char)6);
		SJ.setSuit(4);
		SJ.setRank(11);
		Deck.add(SJ);

		Card S10 = new Card();
		S10.setName("10" + (char)6);
		S10.setSuit(4);
		S10.setRank(10);
		Deck.add(S10);

		Card S9 = new Card();
		S9.setName(" 9" + (char)6);
		S9.setSuit(4);
		S9.setRank(9);
		Deck.add(S9);

		Card S8 = new Card();
		S8.setName(" 8" + (char)6);
		S8.setSuit(4);
		S8.setRank(8);
		Deck.add(S8);

		Card S7 = new Card();
		S7.setName(" 7" + (char)6);
		S7.setSuit(4);
		S7.setRank(7);
		Deck.add(S7);

		Card S6 = new Card();
		S6.setName(" 6" + (char)6);
		S6.setSuit(4);
		S6.setRank(6);
		Deck.add(S6);

		Card S5 = new Card();
		S5.setName(" 5" + (char)6);
		S5.setSuit(4);
		S5.setRank(5);
		Deck.add(S5);

		Card S4 = new Card();
		S4.setName(" 4" + (char)6);
		S4.setSuit(4);
		S4.setRank(4);
		Deck.add(S4);

		Card S3 = new Card();
		S3.setName(" 3" + (char)6);
		S3.setSuit(4);
		S3.setRank(3);
		Deck.add(S3);

		Card S2 = new Card();
		S2.setName(" 2" + (char)6);
		S2.setSuit(4);
		S2.setRank(2);
		Deck.add(S2);

		Card DA = new Card();
		DA.setName(" A" + (char)4);
		DA.setSuit(3);
		DA.setRank(14);
		Deck.add(DA);

		Card DK = new Card();
		DK.setName(" K" + (char)4);
		DK.setSuit(3);
		DK.setRank(13);
		Deck.add(DK);

		Card DQ = new Card();
		DQ.setName(" Q" + (char)4);
		DQ.setSuit(3);
		DQ.setRank(12);
		Deck.add(DQ);

		Card DJ = new Card();
		DJ.setName(" J" + (char)4);
		DJ.setSuit(3);
		DJ.setRank(11);
		Deck.add(DJ);

		Card D10 = new Card();
		D10.setName("10" + (char)4);
		D10.setSuit(3);
		D10.setRank(10);
		Deck.add(D10);

		Card D9 = new Card();
		D9.setName(" 9" + (char)4);
		D9.setSuit(3);
		D9.setRank(9);
		Deck.add(D9);

		Card D8 = new Card();
		D8.setName(" 8" + (char)4);
		D8.setSuit(3);
		D8.setRank(8);
		Deck.add(D8);

		Card D7 = new Card();
		D7.setName(" 7" + (char)4);
		D7.setSuit(3);
		D7.setRank(7);
		Deck.add(D7);

		Card D6 = new Card();
		D6.setName(" 6" + (char)4);
		D6.setSuit(3);
		D6.setRank(6);
		Deck.add(D6);

		Card D5 = new Card();
		D5.setName(" 5" + (char)4);
		D5.setSuit(3);
		D5.setRank(5);
		Deck.add(D5);

		Card D4 = new Card();
		D4.setName(" 4" + (char)4);
		D4.setSuit(3);
		D4.setRank(4);
		Deck.add(D4);

		Card D3 = new Card();
		D3.setName(" 3" + (char)4);
		D3.setSuit(3);
		D3.setRank(3);
		Deck.add(D3);

		Card D2 = new Card();
		D2.setName(" 2" + (char)4);
		D2.setSuit(3);
		D2.setRank(2);
		Deck.add(S2);

		Card CA = new Card();
		CA.setName(" A" + (char)5);
		CA.setSuit(2);
		CA.setRank(14);
		Deck.add(CA);

		Card CK = new Card();
		CK.setName(" K" + (char)5);
		CK.setSuit(2);
		CK.setRank(13);
		Deck.add(CK);

		Card CQ = new Card();
		CQ.setName(" Q" + (char)5);
		CQ.setSuit(2);
		CQ.setRank(12);
		Deck.add(CQ);

		Card CJ = new Card();
		CJ.setName(" J" + (char)5);
		CJ.setSuit(2);
		CJ.setRank(11);
		Deck.add(CJ);

		Card C10 = new Card();
		C10.setName("10" + (char)5);
		C10.setSuit(2);
		C10.setRank(10);
		Deck.add(C10);

		Card C9 = new Card();
		C9.setName(" 9" + (char)5);
		C9.setSuit(2);
		C9.setRank(9);
		Deck.add(C9);

		Card C8 = new Card();
		C8.setName(" 8" + (char)5);
		C8.setSuit(2);
		C8.setRank(8);
		Deck.add(C8);

		Card C7 = new Card();
		C7.setName(" 7" + (char)5);
		C7.setSuit(2);
		C7.setRank(7);
		Deck.add(C7);

		Card C6 = new Card();
		C6.setName(" 6" + (char)5);
		C6.setSuit(2);
		C6.setRank(6);
		Deck.add(C6);

		Card C5 = new Card();
		C5.setName(" 5" + (char)5);
		C5.setSuit(2);
		C5.setRank(5);
		Deck.add(C5);

		Card C4 = new Card();
		C4.setName(" 4" + (char)5);
		C4.setSuit(2);
		C4.setRank(4);
		Deck.add(C4);

		Card C3 = new Card();
		C3.setName(" 3" + (char)5);
		C3.setSuit(2);
		C3.setRank(3);
		Deck.add(C3);

		Card C2 = new Card();
		C2.setName(" 2" + (char)5);
		C2.setSuit(2);
		C2.setRank(2);
		Deck.add(C2);

		Card HA = new Card();
		HA.setName(" A" + (char)3);
		HA.setSuit(1);
		HA.setRank(14);
		Deck.add(HA);

		Card HK = new Card();
		HK.setName(" K" + (char)3);
		HK.setSuit(1);
		HK.setRank(13);
		Deck.add(HK);

		Card HQ = new Card();
		HQ.setName(" Q" + (char)3);
		HQ.setSuit(1);
		HQ.setRank(12);
		Deck.add(HQ);

		Card HJ = new Card();
		HJ.setName(" J" + (char)3);
		HJ.setSuit(1);
		HJ.setRank(11);
		Deck.add(HJ);

		Card H10 = new Card();
		H10.setName("10" + (char)3);
		H10.setSuit(1);
		H10.setRank(10);
		Deck.add(H10);

		Card H9 = new Card();
		H9.setName(" 9" + (char)3);
		H9.setSuit(1);
		H9.setRank(9);
		Deck.add(H9);

		Card H8 = new Card();
		H8.setName(" 8" + (char)3);
		H8.setSuit(1);
		H8.setRank(8);
		Deck.add(H8);

		Card H7 = new Card();
		H7.setName(" 7" + (char)3);
		H7.setSuit(1);
		H7.setRank(7);
		Deck.add(H7);

		Card H6 = new Card();
		H6.setName(" 6" + (char)3);
		H6.setSuit(1);
		H6.setRank(6);
		Deck.add(H6);

		Card H5 = new Card();
		H5.setName(" 5" + (char)3);
		H5.setSuit(1);
		H5.setRank(5);
		Deck.add(H5);

		Card H4 = new Card();
		H4.setName(" 4" + (char)3);
		H4.setSuit(1);
		H4.setRank(4);
		Deck.add(H4);

		Card H3 = new Card();
		H3.setName(" 3" + (char)3);
		H3.setSuit(1);
		H3.setRank(3);
		Deck.add(H3);

		Card H2 = new Card();
		H2.setName(" 2" + (char)3);
		H2.setSuit(1);
		H2.setRank(2);
		Deck.add(H2);

	}

	public void deal() {
		for (int i = 0; i < 5; i++) {
			double pd = Math.random() * Deck.size();
			int pi = (int)pd;
			playerHand.add(Deck.get(pi));
			Deck.remove(pi);
		}			
	}

	public void showHand() {
		System.out.println("Unsorted hands");
		System.out.println("Player" + "\t" + "Suit");
		for (int i = 0; i < playerHand.size(); i++) {
			System.out.println(playerHand.get(i).getName() + "\t" + playerHand.get(i).getSuit());
		}

		System.out.println();

		int numNonSpades = 0;

		for (int i = 0; i < playerHand.size(); i++) {
			if (playerHand.get(i).getSuit() < 4) {
				numNonSpades++;
				playerHand.remove(i);
			}
		}

		System.out.println("Number of cards with Non-Spades: " + numNonSpades);
		System.out.println("Number of cards left in hand: " + playerHand.size());
		
	}
} 

And here's the main method:

import java.util.*;

public class test {

	public static void main (String [] args) {
		Deck d = new Deck();
		d.setUpDeck();
		d.deal();
		d.showHand();
	}
}

I'm consistently getting wrong counts. For example, I get
6 of spades
7 of hearts
8 of hearts
7 of diamonds
8 of diamonds

but it counts 2 cards as non Spades (as oppoesed to 4).

Any ideas?

[Jeff Verdegan]

Let's say you have 5 cards in your hand, call them A, B, C, D, E, and you remove the card at index 2.

What does your hand look like now?

When you go around for the next pass at your loop, which card will you be looking at?

Aside from that, there are a number of problems in your code.

1. You should be using enums, not ints for rank and suit.

2. Even if you're not allowed to use enums, you shouldn't be constructing each card explicitly. Use loops.

3. Use java.util.Random.nextInt(), not Math.random().

4. Never use get(i) to iterate over a Collection. Use a foreach loop, or, if you want to modify the Collection during the iteration, use an Iterator. (That would fix the non-spade-count problem, if I read that bit of your code correctly.)

[Jeremy Graham]

I can't seem to figure out how to make unique objects for an arraylist using a for loop... Any ideas how?

Thanks

[dennis deems]

Jeremy Graham wrote:I can't seem to figure out how to make unique objects for an arraylist using a for loop... Any ideas how?

Thanks

If the instantiations occur inside a loop, they will be unique. No additional effort is required from you to make it so. Have you learned about scope yet? Every set of {} braces defines a scope, and anything local to that scope is unique. If I write

{ String unique = new String("unique");}

and copy/paste that block ten times inside a method, then I have instantiated ten unique String objects. Each of those objects is only visible inside its scope, which is why I don't get an error when I reuse the variable name unique. We can arbitrarily define blocks inside a method, though it's usually not a good idea. However, structures like loops and if/else alternations define their own scope blocks. If I take my String instantiation and put it inside a loop that iterates ten times, then it is just as if I had copied and pasted the block ten times: ten unique Strings are created.

So: you already know how to instantiate an object in a method and add it to a list. Doing it inside a loop isn't any different. Make a for loop that iterates over the ranks, and inside that loop make another, nested loop, that iterates over the suits. Instantiate a card of that rank and suit, then add it to the deck -- much like you already do. The only real difference is that instead of 52 instantiations you only have to write one.

[Jeff Verdegan]

Dennis Deems wrote:
So: you already know how to instantiate an object in a method and add it to a list. Doing it inside a loop isn't any different.

Well, there will need to be special handling for J, Q, K, A, but there are various ways to deal with that.

[Campbell Ritchie]

Jeff Verdegan wrote: . . . special handling for J, Q, K, A, but there are various ways to deal with that.

And making Suit and Rank into enums makes it much easier to find one of those “various ways”.

[Alan Smith]

Give your card class a list for ranks and a list for suits ie:

//in Card class

static String[] suits = {"Clubs", "Diamonds", "Hearts", "Spades"}; //or {"C","D","H","S"} whatever you prefer.
static String[] ranks = {"2", "3", "4", "5", "6", "7", "8", "9", "10", "Jack", "Queen", "King", "Ace"};

then use nested for loops to populate your deck with cards based on the rank and suit arrays. Make sure the card class has rank and suit attributes and be able to set them using the constructer ie:

public Card(String suitIn, String rankIn)

that should get you started.

[Campbell Ritchie]

I still think the earlier suggestion about enums was good. If you are up to reading the Java Language Specification and about the Enum class, that will provide more useful information.

[dennis deems]

Campbell Ritchie wrote:I still think the earlier suggestion about enums was good. If you are up to reading the Java Language Specification and about the Enum class, that will provide more useful information.

Enums would definitely simplify this problem. But I don't think the JLS makes beginner-friendly reading. The Java Tutorial you linked to has everything the beginner needs, and the exercise which follows the Enum chapter is actually this very problem: to represent a deck of cards with enums.

[Jeff Verdegan]

I think it makes sense to do it without enums first, to learn how it can be done with looping and arrays, and then to do it again with enums, as a compare-and-contrast exercise.

[dennis deems]

Jeff Verdegan wrote:I think it makes sense to do it without enums first, to learn how it can be done with looping and arrays, and then to do it again with enums, as a compare-and-contrast exercise.

This is a great idea.

[Winston Gutkowski]

Jeremy Graham wrote:but it counts 2 cards as non Spades (as oppoesed to 4).

So, you have an array of 4 things (Suits). What are their indexes?

Winston

[Jeremy Graham]

Thank you Dennis for your reply. I know I've tried doing that sort of thing in the past, but I apparently was always doing something wrong.

I'm still in the process of figuring out what I want to do with enums. I've tabled it for now, but I'll get back to it.

What is stumping me right now is Jeff's suggestion that I use java.util.Random.nextInt() instead of Math.random(). In trying to use it I'm getting the error: "non-static method nextInt(int) cannot be referenced from a static context". Here's all my code again.

My Card object:
public class Card {

	String cardName;
	int cardRank;
	int cardSuit;
	
	public void setName(String cn) {
		cardName = cn;
	}

	public String getName() {
		return cardName;
	}

	public void setSuit(int cs) {
		cardSuit = cs;
	}

	public int getSuit() {
		return cardSuit;
	}

	public void setRank(int cr) {
		cardRank = cr;
	}

	public int getRank() {
		return cardRank;
	}
}

My Deck2 object (keeping the original Deck object for now):

import java.util.*;

public class Deck2 {
	ArrayList<Card> Deck2 = new ArrayList<Card>();
	ArrayList<Card> playerHand = new ArrayList<Card>();

	static String[] suits = {"" + (char)6, "" + (char)4, "" + (char)5, "" + (char)3};
	static String[] ranks = {" 2", " 3", " 4", " 5", " 6", " 7", " 8", " 9", "10", " J", " Q", " K", " A"};

	public void setUpDeck() {
		for (int r = 0; r < 13; r++) {
			for(int s = 0; s < 4; s++) {
				Card c = new Card();
				c.setName(ranks[r] + suits[s]);
				c.setRank(r);
				c.setSuit(s);
				Deck2.add(c);
			}
		}
	}

	public void deal() {
		for (int i = 0; i < 5; i++) {
			int p = java.util.Random.nextInt(Deck2.size());
			playerHand.add(Deck2.get(p));
			Deck2.remove(p);
		}			
	}

	public void showHand() {
		System.out.println("Player");
		for (int i = 0; i < playerHand.size(); i++) {
			System.out.println(playerHand.get(i).getName());
		}
	}
} 

And the main method:

import java.util.*;

public class test {

	public static void main (String [] args) {
		Deck2 d2 = new Deck2();
		d2.setUpDeck();
		d2.deal();
		d2.showHand();
	}
}

The only static method I see is in that main method.

Help?

[Campbell Ritchie]

Dennis Deems wrote:. . . But I don't think the JLS makes beginner-friendly reading. . . .

No, it doesn’t. That’s why I said, “If you are up to reading . . .”

[Nigel Browne]

Hi Jerremy,

Your Card class doesn't have a constructor. I would create a card with either suit and rank, or name as in the case of a joker.
I would get rid of your set methods as once a card is created it doesn't change its' value or suit.
I would also change your getName method into a toString() method.

I highly recommend that you read the java tutorial regarding enums as your Deck class could be reduced down to around 20 lines of code. Also do the deal and showHand methods really belong in the Deck class?

[Junilu Lacar]

Jeremy Graham wrote:
What is stumping me right now is Jeff's suggestion that I use java.util.Random.nextInt() instead of Math.random(). In trying to use it I'm getting the error: "non-static method nextInt(int) cannot be referenced from a static context".

You wrote:
public void deal() { for (int i = 0; i < 5; i++) { int p = java.util.Random.nextInt(Deck2.size()); playerHand.add(Deck2.get(p)); Deck2.remove(p); } }
Line 24 is the problem. "non-static method nextInt(int)" means that the nextInt method is an instance method. That means you can only call it on an instance of the Random class. How do you get an instance? You use the new keyword, of course.

[Junilu Lacar]

Jeremy Graham wrote:Here's my Card object:
...

One of the complaints about Java which other JVM languages like Scala and Groovy try to avoid is its verbosity in declaring even the simplest of classes. You might want to consider this: Once created, does a Card object ever change its suit or rank? IMO, it doesn't. These are all intrinsic, immutable properties of a Card once it has been instantiated, unless you are simulating a magic card trick where the Card will transform from the Queen of Hearts to the King of Spades right before your eyes

If you agree that a Card should not be able to change its suit or rank once it is instantiated, then you should use a constructor that takes those values and assigns them to final fields. You can then choose to just make the final fields public or provide only getters for those fields, no setters because those don't make sense.

EDIT: Now I feel a little icky... seems I just kicked a Zombie, too

[Winston Gutkowski]

Junilu Lacar wrote:EDIT: Now I feel a little icky... seems I just kicked a Zombie, too

We've all done it.

My only contribution is that I've been playing bridge at club level for about 30 years, and I still can't count cards right.

Winston