How do I make my ArrayList add 52 unique objects instead of all the same?

Question

I've been stuck on this forever, but I think I've realized that my deck builder is ending up making a deck of cards all of the same, last value in the loop.

Here is my code for the deck builder and a hand preparer. It includes an object Card which takes in two integers, here v and s. And an ArrayList of five cards to be dealt. I'd be grateful for some assistance!

public ArrayList buildDeck(){
    ArrayList<Card> newDeck = new ArrayList<Card>(52);     
    for(int v=0;v<13;v++)
        for(int s=0;s<4;s++) {
            Card nextCard = new Card(v,s);          
            newDeck.add(nextCard);
        }
    return newDeck;
}

public ArrayList fiveDeal() {
    ArrayList<Card> handOfFive = new ArrayList<Card>(0);
    for(int d=0;d<5;d++)
    {
        handOfFive.add(cardDeck.get((gen.nextInt(cardDeck.size()))));
    }
    return handOfFive;
}

and here is my Card class

public class Card
{
    public static String value;
    public static String suit;
    public Card()
    {
        value="100";
        suit="Cards";
    }
    public Card(int v, int s)
    {
        if(v==0)
            value="Jack";
        if(v==1)
            value="Ace";
        if(v==11)
            value="Queen";
        if(v==12)
            value="King";
        else
            value = String.valueOf(v);
        if(s==0)
            suit="Clubs";
        if(s==1)
            suit="Spades";
        if(s==2)
            suit="Diamonds";
        else
            suit="Hearts";
    }
    public static String getCard(Object Card)
    {
        return value + " of " + suit;
    }
}

Could you please show the code that prints 52 identical cards to you? Chances are, the error is there :) — Sergey Kalinichenko, Apr 29 '13 at 10:33
Everything seems fine. Post the code where you're actually printing out the whole deck. — Ravi K Thapliyal, Apr 29 '13 at 10:36
@jlordo perhaps it is in the code which deals five cards. I'll edit the post with that code as well. — Riblet, Apr 29 '13 at 10:37
You wouldn't have `v` and `s` as *static* properties in `Card`, now would you? — Marko Topolnik, Apr 29 '13 at 10:38
@MarkoTopolnik: Didn't think of that one... Riblet, post your `Card` class... — jlordo, Apr 29 '13 at 10:40
Not that it should matter here but since you know it's a hand of five do `handOfFive = new ArrayList(5);` instead of `0`. — Ravi K Thapliyal, Apr 29 '13 at 10:43
When you "build your deck" is the output deck meant to be all 52 cards in a random shuffled order or is it in suit and face value order? — Tom 'Blue' Piddock, Apr 29 '13 at 10:43
@MarkoTopolnik: wow, you are a pro and probably a psychic. How did you guess?? I was sure it would turn out to be an empty, eclipse-generated contructor :-) — fdreger, Apr 29 '13 at 10:46
@fdreger From the problem being solved I guessed this was a Java beginner, and I know from previous experience that beginners often make this mistake (I used to teach Java at a university :) — Marko Topolnik, Apr 29 '13 at 10:48
@MarkoTopolnik: If you post an answer, I'll delete mine. The credit should be all yours ;) — jlordo, Apr 29 '13 at 10:49
@jlordo Ah, let it stay :) As a bonus, I'd mention to OP that `public` instance fields are really an advanced approach with many pitfalls, and that beginners should stick to `private` fields and accessor methods. — Marko Topolnik, Apr 29 '13 at 10:51
@jlordo thanks for the help, but I've hit one last snag. I'm trying to use the getCard method in the Card class to print in the client, and it's saying I can't reference it from a static context. — Riblet, Apr 29 '13 at 10:53
@Riblet: just get rid of `static` in the method signature. The method is called on an instance, and not on the class itself. The signature should be `public String getCard()`. — jlordo, Apr 29 '13 at 10:55
@jlordo Sorry, I should have been more clear. I've removed all static in my code aside from public static void main in the client that calls getCard. — Riblet, Apr 29 '13 at 10:56
@Riblet: You need to create a card object, and call the method on it. Like `Card card = new Card(1,2); String s = card.getCard();` — jlordo, Apr 29 '13 at 10:59
Yes, this is precisely how `static` gets in there in the first place... it's that cursed Eclipse QuickFix feature, which many mistake for "spellchecker's suggestion" kind of feature and make an even greater mess than they initially had. — Marko Topolnik, Apr 29 '13 at 10:59
Thank you so much for all of the help everyone! And yes I'm a beginner. This code actually marks the end of my first semester of programming. You guys rock! — Riblet, Apr 29 '13 at 11:07

jlordo · Answer 1 · 2013-04-29T10:54:36.653

As Marko Topolnik anticipated, the error is in your Card class. Change

public static String value;
public static String suit;

to

public String value;
public String suit;

to fix your current problem.

static attributes belong to the class itself, while non-static attributes belong to a specific instance of that class.

Also, I would use the private access modifier instead of public and use getter/setter methods to access the variables (eclipse can generate the methods for you). This way they would be encapsulated and you can control how they are being accessed and modified.

OldCurmudgeon · Answer 2 · 2013-04-29T14:05:22.527

0

I would suggest you create yourself a Deck class that removes the card from the deck when a deal method is invoked.

Apart from the suggestions already made, your issue could be that gen.nextInt(cardDeck.size()) is always returning the same number. However, your problem is that you are trying to model a card deck using an ArrayList without removals.

edited Apr 29 '13 at 14:05

answered Apr 29 '13 at 11:15

OldCurmudgeon

64,482
16
119
213

How do I make my ArrayList add 52 unique objects instead of all the same?

2 Answers2