Why Am I Getting An NPE That Only Appears Occasionally When The Program is Run?

Question

I'm writing a test class for my GiftSelector class using JUnit in BlueJ. When I run the testGetCountForAllPresents() method, I get a NullPointerException on the line:

assertEquals(true, santasSelector.getCountsForAllPresents().get(banana) == 3);

The strange thing about this NPE, is that it rarely appears when I run the test once, but often appears the second time I run the test. It sometimes doesn't appear until I've ran the test 7-8 times consecutively.

The error message I'm getting is: no exception message.

NPE at line 215 in GiftSelectortest.testGetCountForAllPresents

The code for my test class is:

import static org.junit.Assert.*;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

/**
 * The test class GiftSelectorTest. The GiftSelector that you are 
 * testing must have testMode enabled for this class to function. 
 * This is done in the setUp() method.
 */
public class GiftSelectorTest
{
    private GiftList giftList1;
    private GiftList giftList2;
    private GiftList giftList3;
    private Child jack;
    private Child bob;
    private Child dave;
    private Child naughty1;
    private GiftSelector santasSelector;
    private Present banana1;
    private Present orange;
    private Present banana;
    private Present apple;
    private Present bike;
    private Present doll;
    private Present got;
    private Present pearlHarbour;
    private Present dog;
    private Present cat;
    private Present ball;
    private Present heineken;

    /**
     * Default constructor for test class GiftSelectorTest
     */
    public GiftSelectorTest()
    {
        //Nothing to do here...
    }

    /**
     * Sets up the test fixture.
     *
     * Called before every test case method.
     */
    @Before
    public void setUp()
    {
        santasSelector = new GiftSelector();
        santasSelector.setTestMode(true);
        jack = new Child("Jack", 20, "1 A Place", true, true, true, false);
        bob = new Child("Bob", 10, "2 A Place", true, true, true, true);
        dave = new Child("Dave", 10, "3 A Place", true, true, true, true);
        naughty1 = new Child("John", 5, "4 A Place", true, true, true, true);
        giftList1 = new GiftList(jack);
        giftList2 = new GiftList(bob);
        giftList3 = new GiftList(dave);
        banana = new Present("banana", "fruit", 10);
        orange = new Present("orange", "fruit", 10);
        banana1 = new Present("banana", "fruit", 10);
        apple = new Present("apple", "fruit", 10);
        bike = new Present("bike", "toy", 200);
        doll = new Present("doll", "toy", 40);
        got = new Present("game of thrones", "dvd", 50);
        pearlHarbour = new Present("pearl harbour", "dvd", 20);
        dog = new Present("dog", "animal", 100);
        cat = new Present("cat", "animal", 80);
        ball = new Present("ball", "toy", 5);
        heineken = new Present("heineken", "beer", 1.60);
    }

    /**
     * Tears down the test fixture.
     *
     * Called after every test case method.
     */
    @After
    public void tearDown()
    {
        //Nothing to do here...
    }


    @Test
    public void testGetCountForAllPresents()
    {
        System.out.println(santasSelector.getCountsForAllPresents());
        //Test on empty GiftSelector
        assertNull(santasSelector.getCountsForAllPresents());

        //Test on a GiftSelector with one giftlist containing one present
        giftList1.addPresent(banana);
        santasSelector.addGiftList(giftList1);
        System.out.println(santasSelector.getCountsForAllPresents());
        assertEquals(true, santasSelector.getCountsForAllPresents().get(banana) == 1);

        //Test when GiftSelector contains 2 giftlists, each containing the same present object

        giftList2.addPresent(banana);
        santasSelector.addGiftList(giftList2);
        System.out.println(santasSelector.getCountsForAllPresents());
        assertEquals(true, santasSelector.getCountsForAllPresents().get(banana) == 2);

        //Test when GiftSelector contains 3 giftlists, 2 containing the same present object and another containing an identical present but with a different present instance
        giftList3.addPresent(banana1);
        santasSelector.addGiftList(giftList3);
        System.out.println(santasSelector.getCountsForAllPresents());
        assertEquals(true, santasSelector.getCountsForAllPresents().get(banana) == 3); //This is the line I get the NPE

        //Test when GiftSelector contains 3 giftLists, the first with one with a banana, the second with a banana and apple, and the third with a banana1 and ball
        giftList2.addPresent(apple);
        giftList3.addPresent(ball);
        System.out.println(santasSelector.getCountsForAllPresents());
        assertEquals(true, santasSelector.getCountsForAllPresents().get(banana) == 3);
        assertEquals(true, santasSelector.getCountsForAllPresents().get(apple) == 1);
        assertEquals(true, santasSelector.getCountsForAllPresents().get(ball) == 1);

    }


    @Test
    public void testGetMostPopularPresent()
    {
        //Test on empty GiftSelector
        assertNull(santasSelector.getMostPopularPresent());

        //Test on a GiftSelector with one giftList and one Present
        giftList1.addPresent(heineken);
        santasSelector.addGiftList(giftList1);
        assertEquals(true, santasSelector.getMostPopularPresent().comparePresent(heineken));

        //Tset on a GiftSelector with 1 giftList and 2 presents, one more expensive than the other
        giftList1.addPresent(banana);
        assertEquals(true, santasSelector.getMostPopularPresent().comparePresent(banana));

        //Test on a GiftSelector with 1 giftList and 3 presents. Banana and Apple are equal in price, and are both in the top3, 
        //therefore it should return the present closest to the start of the list
        giftList1.addPresent(apple);
        assertEquals(true, santasSelector.getMostPopularPresent().comparePresent(banana) || santasSelector.getMostPopularPresent().comparePresent(apple));

        //Test on a GiftSelector with 2 giftLists, the second list containing banana1, an indentical present to banana
        giftList2.addPresent(banana1);
        santasSelector.addGiftList(giftList2);
        assertEquals(true, santasSelector.getMostPopularPresent().comparePresent(banana));

        //Test on a GiftSelector with 2 giftLists, the first containing four presents and the second containing 2 presents.
        //This tests to see if top3 is working.
        giftList1.addPresent(bike);
        giftList2.addPresent(bike);
        assertEquals(true, santasSelector.getMostPopularPresent().comparePresent(bike));
    }
}

I've included only the test methods that reference the getCountsForAllPresents() method. You'll notice that I've added print statements before each call to an assertEquals() method containing the getCountForAllPresents() method. What's interesting is that before the line where I get the NPE, the print statement prints out the correct value for the HashMap returned by getCountForAllPresents().

The only other strange thing that I've noticed, is that when I go through the testGetCountForAllPresents() method using BlueJ's built in debugger, I notice that giftList3 doesn't appear in the santaMap HashMap in santasSelector, yet the print statement still prints the correct counting, implying it must know about giftList3.

The code for getCountForAllPresents() is:

/**
 * For each present, calculate the total number of children who have asked for that present.
 * 
 * @return - a Map where Present objects are the keys and Integers (number of children requesting
 * a particular present) are the values. Returns null if santaMap is empty.
 */
public HashMap<Present, Integer> getCountsForAllPresents()
{
    if(!santaMap.isEmpty()) {
        //This HashMap contains a mapping from each unique real world present, represented by it's toComparisonString(), to a Present object representing it
        HashMap<String, Present> uniquePresents = new HashMap<String, Present>();
        //This HashMap contains a mapping from each Present object in uniquePresents to the number of times it's toComparisonString() is equal to another in santaMap
        HashMap<Present, Integer> presentFrequency = new HashMap<Present, Integer>();

         for(GiftList wishlist: santaMap.values()) {
            for(Present present: wishlist.getAllPresents()) {
                //Have we already seen this present?
                if(uniquePresents.containsKey(present.toComparisonString())) {
                    //If so, update the count in presentFrequency
                    Integer tmp = presentFrequency.get(uniquePresents.get(present.toComparisonString()));
                    tmp++;
                    presentFrequency.put(uniquePresents.get(present.toComparisonString()), tmp);
                } else {
                    //If not, add it to the maps uniquePresents and presentFrequency (with a frequency of 1)
                    uniquePresents.put(present.toComparisonString(), present);
                    presentFrequency.put(present, 1);
                }
            }
        }
        //Return a map with unique presents as keys and their frequencies as values
        return presentFrequency;
    }
    else {
        //If there are no mappings in Santa's map, return null
        return null;
    }
}

I should explain that santaMap is a HashMap, with a Child object as key and a GiftList object as the value. It basically maps a child to their christmas wishlist. A santaMap can contain only one wishlist by the same child.

I've no idea why I'm getting the NPE, is it something to do with how I've written the getCountForAllPresents() method? How I've implemented the test method/class?

Answer 1

Your Present class does not override hashCode() and equals(). This means that banana1 and banana are two distinct keys in any HashMap that will use them as a key.

So let's see what happens here. You have the banana and the banana1 objects - two of the first, one of the second.

Inside getCountsForAllPresents() you have the two hash maps. The first goes by the comparison string of the objects, and the second - by the objects themselves.

You add the first banana you encounter. If it is the banana object, you'll have something like this:

uniquePresents
banana-fruit-10 ➞ [banana instance]

presentFrequency
[banana instance] ➞ Integer(1)

You continue to iterate. You encounter the next banana object. It's the same object. You'll get:

uniquePresents
banana-fruit-10 ➞ [banana instance]

presentFrequency
[banana instance] ➞ Integer(2)

Now you get to the banana1 object. It's a different object, but it has the same comparison string! What happens?

This condition is true: uniquePresents.containsKey(present.toComparisonString()). This means it goes into the true part of the if.

Integer tmp = presentFrequency.get(uniquePresents.get(present.toComparisonString()));

This means it will take the object currently pointed to by banana-fruit-10, which is the banana object - not the banana1 object, get its associated frequency, and increment it. It also stores by the same object. What you have now is:

uniquePresents
banana-fruit-10 ➞ [banana instance]

presentFrequency
[banana instance] ➞ Integer(3)

Note that presentFrequency does not have a banana1 key at all. And now you return this object.

When you try to retrieve by banana, it works OK - the assert works.

But remember, santaMap itself is a HashMap. This means there is no guaranteed order on it. The iterator may give you giftList1,giftList2,giftList3, but it may also give you giftList3,giftList1,giftList2 - or any other order.

So what happens when it gives you giftList3 first? You`ll end up with:

uniquePresents
banana-fruit-10 ➞ [banana1 instance]

presentFrequency
[banana1 instance] ➞ Integer(3)

Why? Because banana1 was the first gift with the key banana-fruit-10, and that's what it will use from now on.

When this happens, when you try to get banana from the returned object, that key does not exist in the frequency list. It returns null - and there is your NullPointerException.