Java: When adding an object to a tree set exception gets thrown

Question

When try to add an object to my TreeSet, this exception pops up.

Exception in thread "main" java.lang.NullPointerException
    at Circle.compareTo(Shape.java:47)
    at Circle.compareTo(Shape.java:23)
    at java.util.TreeMap.compare(Unknown Source)
    at java.util.TreeMap.put(Unknown Source)
    at java.util.TreeSet.add(Unknown Source)
    at CircleTreeSet.main(CircleTreeSet.java:24)

All I'm doing in my main method is creating the TreeSet, creating a Object, and adding it to the set.

Here is the main method:

class CircleTreeSet {
    public static void main(String[] args) {
        TreeSet<Circle> cs = new TreeSet<Circle>();

        Circle circle1 = new Circle("circle1", 1);

        cs.add(circle1);
    }
}

Here is the class:

class Circle extends Shape implements Comparable<Circle> {
    private static String name;
    private int radius;

    Circle(String n, int r) {
        super(n);
        radius = r;
    }

    public double area() {
        return Math.PI * radius * radius;
    }

    public double perim() {
        return 2 * Math.PI * radius;
    }

    public int compareTo(Circle c) {
        return name.compareTo(c.name);
    }
}

It would be better to show your code in addition to the stack trace of your exception. — Rohit Jain, Aug 11 '13 at 18:47
Could you please check that:- All elements inserted into the set have implement the Comparable interface? — Rahul Tripathi, Aug 11 '13 at 18:49
I'm interested in seeing how your `Circle` has implemented `compareTo`; that is, does it behave correctly when compared to `null`? — Makoto, Aug 11 '13 at 18:50
looks like you have inserted a `null` value into your `TreeMap` and the `compareTo` implementation in `Circle` does not check for `null` values... — PoByBolek, Aug 11 '13 at 18:51
@RaviThapliyal: see this: [compareTo() implementation problems](http://stackoverflow.com/questions/18174791/compareto-implementation-problems) — jlordo, Aug 11 '13 at 18:57
Remove `private static String name;` from Circle as it's hiding the one defined in Shape. — Ravi K Thapliyal, Aug 11 '13 at 19:01
@PoByBolek what your saying is i should have a check in my compareTo method handles when the Tree set checks to make sure there are no duplicates and it just checks against empty slots in the set? — Bungalo Soldier, Aug 11 '13 at 19:04

Makoto · Accepted Answer · 2013-08-11T19:14:46.993

Two thoughts.

I'm not sure what purpose the name variable would actually serve, since there's no way to access it from outside of the class, nor is it used inside the class. If it's not used, remove it.
Your compareTo is simply incorrect. Think of a comparison like this.
- How do I determine equivalence? How are two Circles deemed equivalent?
- How do I determine the natural ordering? In what way are Circles ordered by?

A TreeSet cares about the natural ordering of its elements.

Let's define some laws about Circles, then.

A Circle is equivalent to another Circle if and only if their radius is equivalent.
A Circle is lesser in rank to another Circle if and only if their radius is lesser rank to another Circle.
A Circle is higher in rank to any nonexistent Circle.

Let's move forward with those laws, and define compareTo. To fully complete this, though, we need a getter for radius:

public Integer getRadius() {
    return Integer.valueOf(radius);
}

I'm leveraging Integer instead of int, since Integer is also Comparable. It makes our compareTo a little easier.

public int compareTo(final Circle other) {
    if(other == null) {
        return 1;
    } else {
        return Integer.valueOf(radius).compareTo(other.getRadius());
    }
}

An alternative way, as pointed out in the comments, would also allow you to take the difference of our radius, and the other object's radius, which would satisfy the general contract for Comparable - if the difference is 0, they're equivalent; if it's greater than 0, then it's larger; is it's less than 0, then it's smaller.

To do that, we change our getter to return int:

public int getRadius() {
    return radius;
}

...and modify our compareTo as such:

public int compareTo(final Circle other) {
    if(other == null) {
        return 1;
    } else {
        return radius - other.getRadius();
    }
}

This is just more of an exercise in my programming, I'm a bit rusty and have some upcoming exams — Bungalo Soldier, Aug 11 '13 at 19:07
Any specific reason you use `Integer` instead of `int` for `getRadius()`? — PoByBolek, Aug 11 '13 at 19:08
@PoByBolek: "I'm leveraging Integer instead of int, since Integer is also Comparable. It makes our compareTo a little easier." — Makoto, Aug 11 '13 at 19:08
@Makoto And what's wrong with `return radius - other.getRadius()` if `getRadius()` returns `int`? — PoByBolek, Aug 11 '13 at 19:09
@PoByBolek **Absolutely nothing.** If one wants to avoid autoboxing, then that could be seen as a viable solution. — Makoto, Aug 11 '13 at 19:11
@Makoto but you're probably right. `Integer.compareTo(Integer)` is easier to grasp then subtracting to `int`s for comparison... (at least for me)... But now that you edited your post, also fine :) — PoByBolek, Aug 11 '13 at 19:16

score 2 · Answer 2 · answered Aug 11 '13 at 18:55

2

name is static and is null because it is never set. I think you have misunderstood the meaning of static.

answered Aug 11 '13 at 18:55

Robin Green

32,079
16
104
187

thanks for pointing that out I didn't mean for that to be static at all – Bungalo Soldier Aug 11 '13 at 19:00
Yes, `name` is null, and that's what causes the `compareTo` to blow up. But, does that really fix the issue? – Makoto Aug 11 '13 at 19:18
removing static yielded no benefits but its still an incorrect use of it...thanks – Bungalo Soldier Aug 11 '13 at 19:42

score 0 · Answer 3 · answered Aug 11 '13 at 18:56

The name of your Circle should probably not be static and you should assign a value to it in the Circle constructor:

class Circle extends Shape implements Comparable<Circle> {
    private String name;
    private int radius;

    Circle(String n, int r) {
        this.name = n; // this is crucial
        this.radius = r;
    }

    public int compareTo(Circle other) {
        return name.compareTo(other.getName());
    }

    public String getName() {
        return name;
    }
}

score 0 · Answer 4 · answered Aug 11 '13 at 19:35

You get a NullPointerException, as you never initialize the static variable name. As a result, when you invoke the method compareTo(...), the time the code

return name.compareTo(c.name);

is executed, name has a null value. You are trying to invoke a method (comparedTo(...)) via this null-valued variable, so this will result in a NullPointerException throwing.

Addidionally, having the name set as static will probably result in its value overwriting everytime you create an object. Static variables are considered as "unique" and "common" in a sense among all created objects, as their lifetime extends across the entire run of the program. To take it one step further, static variables are stored in a different place of computer memory than local variables or objects are stored.

Java: When adding an object to a tree set exception gets thrown

4 Answers4