Iterating through an array and printing each object

Question

My array is being loaded and is printing the cards out as planned (in the order they appear in the file). When I try to cycle back through the arraylist in a separate method to check whether or not the data is there, it only prints the last object rather than each of them. Can anybody tell me why?

The load method

public class
    TestFrame {

//VARIABLES
private static Deck deck;
private static Card card;
private static Scanner scan;
private final static String fileName = "cards.txt";
static ArrayList<Card> cards = new ArrayList<>();

private static void Load(){

    deck = new Deck();
    card = new Card();

    // Load in the card file so that we can work with the data from cards.txt internally rather than from the file constantly.

    try(FileReader fr = new FileReader(fileName);
            BufferedReader br = new BufferedReader(fr);
            Scanner infile = new Scanner(br)){

        int numOfCards = infile.nextInt();
        infile.nextLine(); // Do we need this? Yes we do. Illuminati confirmed.
        for(int i=0; i < numOfCards; i++){
            String value = infile.nextLine();
            String suit = infile.nextLine();
            Card newCard = new Card(value, suit);
            card.addCard(newCard);
            System.out.print(newCard.getValue());
            System.out.print(newCard.getSuit());
            System.out.println(" ");
            //Print out the object before cycling through again so we can see if it's working
            //We can use this to add then cards to the shuffle array at a later date
        }

    }

Which prints out this when ran:

ah 
2h 
3h 
4h 
5h 
6h 
7h 
8h 

etc etc. This is the order they're in the .txt file.

I then use these methods to display all of the cards to make sure I can manipulate the data elsewhere

private static void displayAllCards(){
    Card[] cards = Card.getAll();
    for(Card c : cards){
        System.out.print(Card.getValue());
        System.out.print(Card.getSuit());
        System.out.println(" ");
    }
}

and the getAll() method

public static Card[] getAll(){
    Card[] brb = new Card[cards.size()];
    int tempCount = -1;
    for(Card c : cards){
        tempCount++;
        brb[tempCount] = c;
    }
    return brb;
}

When getAll() is ran, it only prints out "ks" (king of spades) which is the last card in the .txt file. Can anybody tell me why this is happening?

Thanks

EDIT: CARD CLASS

package uk.ac.aber.dcs.cs12320.cards;

import java.util.ArrayList;


public class Card {

protected static String value;
protected static String suit;
static ArrayList<Card> cardsList = new ArrayList<>();

public Card(String v, String s){
    this.value = v;
    this.suit = s;
}

public Card() {

}

public static Card[] getAll(){
    Card[] brb = new Card[cardsList.size()];
    int tempCount = -1;
    for(Card c : cardsList){
        tempCount++;
        brb[tempCount] = c;
    }
    return brb;
}

public static void deleteAll(){
    cardsList.clear();
}

public static String getValue() {
    return value;
}

public void setValue(String value) {
    Deck.value = value;
}

public static String getSuit() {
    return suit;
}

public void setSuit(String suit) {
    Deck.suit = suit;
}

public void addCard(Card card){
    cardsList.add(card);
}

}

Answer 1

In Card the variables suit and value are declared static. This means that there is only one variable, shared between all cards.

When you load, you create a card. You set suit and value So they get h and a respectively. And then you print them.

Then you load the next two lines. You create a new card. But the variables are static, not part of the state of the new object, but the same suit and value that contain h and a. The new value is 2. And it replaces the a. Then you print it. The old value is gone.

So your print in the load shows the momentary value of suit and value, but in truth, there is only one value - the last value you loaded from the file.

These variables should be part of the card's state, thus, not static!

Note: your IDE notices when you try to use an instance variable from a static context. For example, your getValue method is static. So it can't access instance variables. However, the correction is not to set the variable to static, but rather to change the method, which logically is supposed to be an instance method, to not static. It may complain again because you are calling the method from static context. Then you have to carefully look why: you are not supposed to use this method statically - you should create an instance of Card and call the method using the variable you created.

So, IDE suggestions to "make it static" are not the correct solution! You should make all instance-related variables and methods not static, and solve "static context" issues by checking why you didn't create an instance and decided to call an instance-related method statically instead.

Answer 2

You are calling getAll() on a specific card, however before when you had card.addCard, you would only be adding one card to the list since every Card has its own ArrayList. This is why it only would print ONE CARD. You need to use getAll() on the ArrayList you made in the file.

Make a getAll() method in the main file that uses static ArrayList<Card> cards = new ArrayList<>();

   public static Card[] getAll(){
    Card[] brb = new Card[cards.size()];
    int tempCount = 0;
    for(Card c : cards){
        brb[tempCount] = c;
        tempCount++;
    }
    return brb;

This will work.

Answer 3

Your card class is not implemented well. The reason you are getting only the last value is because your getters and String variables are static. There is only one copy of static variable per class. You need to make those instance variables/methods and change your print loop to

for(Card c : cards){
    System.out.print(c.getValue());
    System.out.print(c.getSuit());
    System.out.println(" ");
}

See this answer for an elaboration on static vs instance

Answer 4

card.addCard(newCard); looks like it could be cards.add(newCard); and also a tip, changing your indexing looks a bit nicer when you start at zero for the getAll function

    public static Card[] getAll(){
    Card[] brb = new Card[cards.size()];
    int tempCount = -1;
    for(Card c : cards){
        tempCount++;
        brb[tempCount] = c;
    }
    return brb;
}

change it to

    public static Card[] getAll(){
    Card[] brb = new Card[cards.size()];
    int tempCount = 0;
    for(Card c : cards){
        brb[tempCount] = c;
        tempCount++;
    }
    return brb;
}

Answer 5

card.addCard(newCard);

I think this should be cards.addCard(newCard); instead