Array list of objects just creates a reference to the same object java

Question

I am working on homework for school. I am making a simplified deck of cards, you can choose how many suits and how many cards per suit. (suit and rank). I have a Card class that creates a single card and a DeckOfCards class that creates a deck of Cards (a deck of Card Objects). I am attempting to put the cards into an ArrayList (inside the DeckOfCards constructor), but every time it just creates a reference to the most recent card created. I have spent a couple hours trying to figure it out and I cant find an answer in any search.



public class DeckOfCards
{

    private int counter = 0;
    private ArrayList<Card> cardList = new ArrayList<>();

    public DeckOfCards(int rank, int suit)
    {
        for (int x = 0; x < suit; x++) // x is suit
        {
            for (int y = 0; y < rank; y++)  // y is rank
            {
                cardList.add(counter, new Card(x, y));
                counter++; // counter is position in ArrayList / deck
            }
        }
    }

    public String dealCard(int numOfCards)
    {
        // returns the card (numOfCards)
        return cardList.get(numOfCards).toString();
    }
}

/* Card Class and Constructor
public class Card
{
    private static int SUIT;
    private static int RANK;

    public Card(int suit, int rank)
    {
        this.SUIT = suit;
        this.RANK = rank;
    }

    public String toString()
    {
        return ("S"+ SUIT + "R" + RANK);
    }
}



Depending on the rank and suit the output should be

S1R1
S1R2
S1R3
.
.
.
S4R1
S4R2
S4R3

But the out put is always the last card created
S4R3

Because fields in your `Card` class are static. So it is not reference to the last card added to deck of cards, but all your cards have same values of fields because you declared them as `static` — Michał Krzywański, Jun 20 '19 at 16:15

QuaternionsRock · Answer 1 · 2019-06-20T16:34:55.237

0

ArrayList.get(int index) is your problem. Your dealCard method is passing the same numOfCards variable (I assume 1, 2, 3, etc. in your test cases) as an index within the list. What you should be doing depends on the behavior that you're looking for.

Should the cards be shuffled? If so, take a look at this.

Should the cards be removed from the deck once dealt? If so, you should be using ArrayList.remove(int index), which both removes the card from the deck and returns the card at the same time.

Besides that, your method should look something like this:

public String dealCard(int numOfCards)
{
    Card[] dealtCards = new Card[numOfCards]; //This could also be another ArrayList if you want, but unless you're going to be adding/removing cards from the returned object afterwards it shouldn't be necessary
    for(int i = 0; i < numOfCards; i++) {
        dealtCards[i] = cardList.get(i); //or cardList.remove(0);
    }
    return dealtCards; //Notice that this will return the same cards each time you call the method if you use cardList.get(i) unless you implement a cyclical counting variable outside of the method and use it inside the method.
}

edited Jun 20 '19 at 16:34

answered Jun 20 '19 at 16:25

QuaternionsRock

832
7
14

He has overriden `toString()` method, and it is not a problem with accesing the arrayList with index - you do not even know how he invokes this method because he has not shown his code. More likely it is a problem with static fields in his Card class. – Michał Krzywański Jun 20 '19 at 16:34
@michalk you're right about the `toString()` bit, I removed that part of my answer. You're also right that those vars shouldn't be static, but `return cardList.get(numOfCards).toString();` definitely doesn't make any sense unless `numOfCards` is a typo for `numOfCard`. Which I am now realizing is a strong possibility. – QuaternionsRock Jun 20 '19 at 16:38
Yeah, you called that, numOfCards is a typo, it is within a loop to display a varying number of cards, hence I named it numOfCards instead of singular by mistake. The main problem was the static variable type. You people are good and fast. Thank you for your input. – Inno Jun 20 '19 at 16:47

score 0 · Answer 2 · answered Jun 20 '19 at 16:43

If you want to print the your cards as your creating them you can simply call your toString() method from your Card() constructor...

...but if your looking to print them from deal cards then you need to index through each card in the arraylist using either a for loop or through recursive calls (Which you may already be doing outside the dealCard method).

I think the reason you're only printing the most recent card is because your parameter - 'numOfCards' is the 'actual' number of cards you created - which is also the last index in your arraylist and may be why you are only printing the your most recently created card.

PS I don't think you need to be using a counter to represent the index of your arraylist. Using arraylist.add('object') simply appends the object parameter to the end of the list. There is already an index of the objects within the arraylist so using a counter sorta defeats the purpose.

Array list of objects just creates a reference to the same object java

2 Answers2