Hi again, I posted last week about a Go Fish game that I was trying to write - well I've sorted out my past issues and now I'm trying to rewrite my CardPile class (that defines such things as the deck or a player's hand) to consist of an ArrayList of Card objects, rather than an Array. I've gotten the class to compile and I'm getting a runtime error that I think traces back to the searchValue method of the following class:

import java.util.Random;
import java.util.List;
import java.util.ArrayList;

public class CardPile {
    private ArrayList<Card> cards = new ArrayList<Card>();

    private static Random r = new Random(1);

    public void addToBottom(Card c) {
        if (cards.size() == 52) {
            System.out.println("The CardPile is full. You cannot add any more Card objects.");
        }
        this.cards.add(c);
    }

    public Card removeCard(Card c) {
        if (this.cards.contains(c)) {
            this.cards.remove(c);
        }
        return null;
    }

    public Card removeTop() {
        return this.cards.remove(0);
    }

    public int searchValue(int value) {
        int count = 0;
    //I know there is an ArrayList method that gives me the first/last
    //indices of a specified Object, but we're just looking for the value-
    //aspect of the Card, not the suit, so I decided to stick with the old logic
        for (int i = 0;i < cards.size();i++) {
            if (this.cards.get(i).getValue() == value) {
                count++;
            }
        }
        return count;
    }

    public Card[] removeAll(int value) {
        int count = searchValue(value);
        Card[] removed = new Card[count];
        for (int i = 0;i < this.cards.size();i++) {
            if (this.cards.get(i).getValue() != value) {
                this.cards.remove(i);
            }
        }
        this.cards.toArray(removed);
        return removed;
    }

    public int getNumberCards() {
        return this.cards.size();
    }

    public String toString() {
        if (this.cards.isEmpty()) {
            return "";
        }
        String builder = "";
        for (int i = 0;i < this.cards.size();i++) {
            builder = builder + this.cards.get(i) + ", ";
        }
        builder = builder + this.cards.get(this.cards.size() - 1);
        return builder;
    }

    public void shuffle() {
        if (this.cards.isEmpty()) {
            return;
        }
        for (int count = 0; count < 100000;count++) {
            int i = r.nextInt(this.cards.size());
            int j = r.nextInt(this.cards.size());
            Card temp = this.cards.get(i);
            this.cards.set(i, this.cards.get(j));
            this.cards.set(j, temp);
        }
    }

    public static CardPile makeFullDeck() {
        CardPile deck = new CardPile();
        for (int suit = 0;suit < 4;suit++) {
            for (int value = 1; value < 14;value++) {
                deck.addToBottom(new Card(suit, value));
            }
        }
        deck.shuffle();
        return deck;
    }
}

Any idea what's wrong with that method? I feel like it's a matter of me not fully understanding the properties of an ArrayList, but I really don't know where to start...

The main program (GoFish) runs, it asks the user to specify the number of players, then the following error comes up:

Exception in thread "main" java.lang.NullPointerException
    at CardPile.searchValue(CardPile.java:34)
    at CardPile.removeAll(CardPile.java:42)
    at Player.removeAll(Player.java:20)
    at GoFish.updateBooks(GoFish.java:120)
    at GoFish.main(GoFish.java:16)

That's what I get when I try to select 3, 5, or 6 players. If I choose 2 or 4 players then it goes an additional step in the main method (asks me to query another player for a card) except it doesn't print my hand as it's supposed to (so I don't know which card to pick), and following my arbitrary card/player selection it then crashes giving the above error again. I was hoping I wouldn't have to post all of the classes, but let me know if I should...

So you are getting an NPE in this line?
this.cards.get(i).getValue() == value
you first need to print out the parts of that expression to see what's null, is is cards? or the result of calling cards.get(i)? or the result of calling cards.get(i).getValue()?
Once you know what is null you can start to back-track printing values as you go to find out why it's null.

So here's my new searchValue method:

public int searchValue(int value) {
        int count = 0;
    //I know there is an ArrayList method that gives me the first/last
    //indices of a specified Object, but we're just looking for the value-
    //aspect of the Card, not the suit, so I decided to stick with the old logic
        for (int i = 0;i < cards.size();i++) {
            if (this.cards.get(i).getValue() == value) {
                System.out.println(this.cards.get(i));
                System.out.println(this.cards.get(i).getValue());
                count++;
            }
        }
        return count;
    }

Every time it prints the same thing:

Ace of Clubs
1

So then I thought maybe something was wrong with the makeFullDeck method, so I added the following lines to that method to print off a card count to ensure that the full deck was being made correctly:

int cardCount = suit*13 + value;
System.out.println(cardCount);

This prints off the numbers 1-52, so that seemed okay but I was still suspicious of that method. Next I added deck.toString(); in the main method of the GoFish class (which actually runs the game logic) following the calling of the makefullDeck method, and it only prints the same card as before:

Ace of Clubs
1

So I've deduced that the error lies in either the makeFullDeck method or the toString method. Or both... What's weird is that in my suit key, Clubs is assigned the value of 2, so I don't get why the Ace of Clubs (which is Card(1, 2)) keeps showing up. I would expect Card(1, 0) - Ace of Spades, because all of my loops begin at the lowest suit/value ints. Unless the shuffle method is producing the same card order each time and Ace of Clubs happens to land in that position - in which case my shuffle method needs to be fixed, but I thought it was alright...

the error lies in

What error is that? An exception or something wrong with the logic so that the code gives the wrong results?

Edited 4 Years Ago by NormR1

The original error. I know the NPE is only being traced back to my searchValue method, but I think it actually stems from some incorrect logic in either/both of the other two methods I mentioned (makeFullDeck or toString)

If you are getting a NPE, what variable has the null value? You need to find that out before you can on Why doesn't that variable have a non-null value?

How do I figure that out? Here's what I've added:

    public int searchValue(int value) {
        int count = 0;
    //I know there is an ArrayList method that gives me the first/last
    //indices of a specified Object, but we're just looking for the value-
    //aspect of the Card, not the suit, so I decided to stick with the old logic
        for (int i = 0;i < cards.size();i++) {   
            System.out.println(this.cards.size());
            if (this.cards.get(i).getValue() == value) {
                boolean tester = this.cards.get(i).getValue() == value;
                count++;
                System.out.println(tester);
            }
        } 
        return count;
    }

It prints the following (starting from the beginning of the game):

How many players?
3
7
7
7
7
7
7
true
7
4
4
4
4
Exception in thread "main" java.lang.NullPointerException
    at CardPile.searchValue(CardPile.java:35)
    at CardPile.removeAll(CardPile.java:45)
    at Player.removeAll(Player.java:20)
    at GoFish.updateBooks(GoFish.java:120)
    at GoFish.main(GoFish.java:16)

So it asks "How many players?", and I enter 3. Then the loop prints the player's hand size, and "true" if card with value = i is found in the hand. The program crashes at i = 12, I guess...

Exception in thread "main" java.lang.NullPointerException
at CardPile.searchValue(CardPile.java:35)

Look at line 35. What variable on that line has a null value? You need to find it and then find why it does not have a valid value.

You should also print out the values of i and cards.get(i) on line 7 replacing what is there.

The following line should be moved outside of the loop since the value of size() is not changed.

System.out.println(this.cards.size());

Edited 4 Years Ago by NormR1

K, this is what I get when I do what you suggested:

How many players?
3
0. Jack of Spades
1. 4 of Spades
2. 5 of Hearts
3. 3 of Hearts
4. 6 of Clubs
5. Ace of Clubs
6. 7 of Spades
0. 4 of Spades
1. 3 of Hearts
2. Ace of Clubs
3. null
Exception in thread "main" java.lang.NullPointerException
    at CardPile.searchValue(CardPile.java:35)
    at CardPile.removeAll(CardPile.java:45)
    at Player.removeAll(Player.java:20)
    at GoFish.updateBooks(GoFish.java:120)
    at GoFish.main(GoFish.java:16)

So it looks like the Jack, 5, 6, and 7 cards have been removed from the hand, decreasing the number of cards from 7 to 4 like we saw previously, then the 4th card (i = 3) in the hand throws the NPE. I don't get why it's doing this... Must be something wrong with my loop, but why are cards being removed?

There must be something else the code does that is not shown in what has been posted.
Does the code add null values to cards?

Another debugging aid - print out cards: println("cards="+cards) to show all the entries in cards.

Edited 4 Years Ago by NormR1

The program consists of 4 .java files, except I have two version of the CardPile class: one written with a Card Array, the other written with a Card ArrayList. The former runs perfectly with the other three classes, so I know that the issue lies in my new version of the CardPile class.
The code should only be "removing" cards (i.e. assigning that index a null value) from a player's hand when there are four-of-a-kind (i.e. a "book" in Go Fish). It looks like cards are being inappropriately removed for some reason...

Did you try printing cards? What does it show?
For example:

      ArrayList al = new ArrayList();
      al.add(1);
      al.add(null);  // <<<<< NOTE possible to have a null element !!!!
      System.out.println("al=" + al + " 1=" + al.get(1));   // al=[1, null] 1=null

If you go past the end of the arraylist you'll get this:
java.lang.IndexOutOfBoundsException: Index: 2, Size: 2

Edited 4 Years Ago by NormR1

This is what I get when I print 'cards':

How many players?
3
cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
cards=[4 of Spades, 3 of Hearts, Ace of Clubs, null]
cards=[4 of Spades, 3 of Hearts, Ace of Clubs, null]
cards=[4 of Spades, 3 of Hearts, Ace of Clubs, null]
cards=[4 of Spades, 3 of Hearts, Ace of Clubs, null]
Exception in thread "main" java.lang.NullPointerException

So before the 8th iteration of the loop, Card objects are being removed from the 'cards' ArrayList. That shouldn't be happening though, because my loop should terminate after the 7th iteration

How does the null element get in the arraylist?
Removing an element does not leave a null element. I think the null element has to be added.
Where do you add elements to the arraylist? You need to find where you are adding a null element. Add some more printlns of cards to catch where in the code the null is coming from. That make take a few iterations to locate the code.

K, so I've added a println(cards) in both the removeAll method (numbered "1"), and the searchValue method (numbered "2") to see where stuff is messing up. I know it has to be in one of those two methods (according to the error stack trace).

How many players?
3
1. cards =[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
2. cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
2. cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
2. cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
2. cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
2. cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
2. cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
2. cards=[Jack of Spades, 4 of Spades, 5 of Hearts, 3 of Hearts, 6 of Clubs, Ace of Clubs, 7 of Spades]
1. cards =[4 of Spades, 3 of Hearts, Ace of Clubs, null]
2. cards=[4 of Spades, 3 of Hearts, Ace of Clubs, null]
2. cards=[4 of Spades, 3 of Hearts, Ace of Clubs, null]
2. cards=[4 of Spades, 3 of Hearts, Ace of Clubs, null]
2. cards=[4 of Spades, 3 of Hearts, Ace of Clubs, null]
Exception in thread "main" java.lang.NullPointerException
    at CardPile.searchValue(CardPile.java:35)
    at CardPile.removeAll(CardPile.java:44)
    at Player.removeAll(Player.java:20)
    at GoFish.updateBooks(GoFish.java:120)
    at GoFish.main(GoFish.java:16)

I don't understand why removeAll is setting null elements and removing cards - or at least I think it's removeAll... I could be misinterpreting the results

What code is executed between lines 10 and 11? Line 11 is where null first appears in the arraylist. Add an id String to the println so you know where each line was printed:

println("<ID HERE> " + cards)

have the <ID HERE> String be unique in every println

Here's an easy way to find out exactly where you are adding a null...
Add this class to your project

class DebugArrayList<T> extends ArrayList<T> {
   // same as ArrayList, but throws exception if you add a null
   @Override
   public boolean add (T value){
      if (value == null) throw new NullPointerException("adding null");
      return super.add(value);
   }
}

change
private ArrayList<Card> cards = new ArrayList<Card>();
to
private ArrayList<Card> cards = new DebugArrayList<Card>();

run program again.

(you may need to do the same thing with add(int, T) and addAll if you are using those methods)

Edited 4 Years Ago by JamesCherrill

I just solved my problem - or I should say problems... It turns out I was just getting ahead of myself and made several stupid mistakes. Thank you for all your help!

I just got cocky and thought I could rewrite a couple methods to be more "elegant" than the original design, but I ended up making a few small errors in the process. Here's the final code:

import java.util.Random;
import java.util.List;
import java.util.ArrayList;

public class CardPile {
    private ArrayList<Card> cards = new ArrayList<Card>();

    private static Random r = new Random(1);

    public void addToBottom(Card c) {
        if (this.cards.size() == 52) {
            System.out.println("The CardPile is full. You cannot add any more Card objects.");
        }
        this.cards.add(c);
    }

    public Card removeCard(Card c) {
        if (this.cards.contains(c)) {
            this.cards.remove(c);
        }
        return null;
    }

    public Card removeTop() {
        return this.cards.remove(0);
    }

    public int searchValue(int value) {
        int count = 0;
    //I know there is an ArrayList method that gives me the first/last
    //indices of a specified Object, but we're just looking for the value-
    //aspect of the Card, not the suit, so I decided to stick with the old logic
        //See below method for explanation of following line
        //System.out.println("2. cards=" + cards);
        for (int i = 0;i < this.cards.size();i++) {  
            if (this.cards.get(i).getValue() == value) {
                count++;
            }
        } 
        //System.out.println("Count = "+count);
        return count;
    }

    public Card[] removeAll(int value) {
        //I was having a lot of trouble with my removeAll and searchValue methods
        //but they turned out to be stupid mistakes… I left the following "print check"
        //line in anyway to show how I narrowed down my problem
        //System.out.println("(removeAll) cards ="+ cards);
        int count = searchValue(value);
        Card[] removed = new Card[count];
        int deletedCount = 0;
        int i = 0;
        while (deletedCount < count) {
            if (this.cards.get(i).getValue() == value) {
                removed[deletedCount] = this.cards.remove(i);
                deletedCount++;
            } else {
                i++;
            }
        }
        return removed;
    }

    public int getNumberCards() {
        return this.cards.size();
    }

    public String toString() {
        if (this.cards.isEmpty()) {
            return "";
        }
        String builder = "";
        for (int i = 0;i < this.cards.size() - 1;i++) {
            builder = builder + this.cards.get(i) + ", ";
        }
        builder = builder + this.cards.get(this.cards.size() - 1);
        return builder;
    }

    public void shuffle() {
        if (this.cards.isEmpty()) {
            return;
        }
        for (int count = 0; count < 100000;count++) {
            int i = r.nextInt(this.cards.size());
            int j = r.nextInt(this.cards.size());
            Card temp = this.cards.get(i);
            this.cards.set(i, this.cards.get(j));
            this.cards.set(j, temp);
        }
    }

    public static CardPile makeFullDeck() {
        CardPile deck = new CardPile();
        for (int suit = 0;suit < 4;suit++) {
            for (int value = 1; value <= 13;value++) {
                deck.addToBottom(new Card(suit, value));
            }
        }
        deck.shuffle();
        return deck;
    }
}
Comments
Good feedback. Thanks
This question has already been answered. Start a new discussion instead.