Java TreeSet not adding object

Question

I am trying to add objects into a Treeset but the objects not all are getting added.

class Fruits
{
     String name ;
     int weight;
     int price;

    Fruits(String n, int w, int p)
    { 
        this.name=n;
        this.weight=w;
        this.price =p;
    }

    @Override
    public int hashCode() {
        System.out.println("hashcode called");
        int prime =31;
        int result =1;
        result = prime*result +(this.name.hashCode()+this.price+this.weight);
        return result;
    }

    @Override
    public boolean equals(Object obj) {
        System.out.println("Equals called");
        if(null!=obj)
        {
            Fruits f= (Fruits) obj;
            if(this.name.equals(f.name) && this.price==f.price && this.weight == f.price)
            {
                return true;
            }
        }
        return false;
    }
}

class FruitsComparator implements Comparator<Fruits>
{
    //Order by Name, then quanity and then Price
    @Override
    public int compare(Fruits f1, Fruits f2)
    {
        if(f1.name.equals(f2.name) && f1.weight == f2.weight && f1.price == f2.price)
        {
            System.out.println(1);
            return 0;
        }
        else if(f1.name.equals(f2.name) && f1.weight==f2.weight && f1.price < f2.price)
        {
            System.out.println(2);
            return -1;
        }
        else if (f1.name.equals(f2.name) && f1.weight==f2.weight && f1.price > f2.price)
        {
            System.out.println(3);
            return 1;
        }
        else if (f1.name.equals(f2.name) && f1.weight<f2.weight && f1.price == f2.price)
        {
            System.out.println(4);
            return -1;
        }
        else if (f1.name.equals(f2.name) && f1.weight>f2.weight && f1.price == f2.price)
        {
            System.out.println(5);
            return 1;
        }
        else if (f1.name.compareTo(f2.name) <1 && f1.weight==f2.weight && f1.price == f2.price)
        {
            System.out.println(6);
            return -1;
        }
        else if (f1.name.compareTo(f2.name) >1 && f1.weight==f2.weight && f1.price == f2.price)
        {
            System.out.println(7);
            return 1;
        }
            return 0;
    }       
}

From public static void main of another class.

Fruits f1= new Fruits("Apple",1,3);
Fruits f2= new Fruits("Apple",10,1);
Fruits f3= new Fruits("Apple",15,2);
Set<Fruits> sf = new TreeSet<Fruits>(new FruitsComparator());
sf.add(f1);
sf.add(f2);
sf.add(f3);
System.out.println("--Fruits Example--");
for( Fruits f: sf)
{
    System.out.println(f.name+"-"+f.weight+"-"+f.price);
}

The output I get is :

--Fruits Example--
Apple-1-3

But when I have fruits objs as below i get the all the objects just keeping everything same but the third element. Fruits f1= new Fruits("Apple",1,3); Fruits f2= new Fruits("Apple",1,1); Fruits f3= new Fruits("Apple",1,2);

The output get for this is

--Fruits Example--
Apple-1-1
Apple-1-2
Apple-1-3

So somehow my objects are treated as same when I keep different elements on weight and price. I couldn't figure out as why the objects are treated as same. Please help.

Voting to close as a copy-paste error (f.price should be f.weight). — Sergey Kalinichenko
– Sergey Kalinichenko, Commented Jun 22, 2016 at 18:00
Your comparator implementation is so confusing as to be unreadable, let alone fixable. Also, .compareTo results should be compared to 0, not 1. — Louis Wasserman
– Louis Wasserman, Commented Jun 22, 2016 at 18:30

JackDaniels · Accepted Answer · 2016-06-22 20:00:45Z

3

The primary issue is, you are always checking two fields to be equal and only one to be different. At the final else, that happens if at least 2 fields are different, you return 0 which means they should be treated as equal, and that is the reason you have this issue.

Since the order you want is to first sort by name, then by quantity and then by price, remove the && f1.price == f2.price from the 4th condition onwards, and remove && f1.weight==f2.weight on the last two.

You can avoid this issue completely if you use Java 8 style.

Set<Fruits> sf = new TreeSet<Fruits>(Comparator.comparing(Fruits::getName)
    .thenComparing(Fruits::getWeight)
    .thenComparing(Fruits::getPrice)
    );

I have added the working code in codiva - online java compiler ide. I have also included a slightly cleaner implementation in FruitsComparator.java file.

edited Jun 22, 2016 at 20:00

answered Jun 22, 2016 at 19:20

JackDaniels

1,09010 silver badges31 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Vineet · Accepted Answer · 2016-06-22 18:26:40Z

1

Tree related collections don't use equals() or hashCode(). Those come into play for Map.

Your conditions in the compare result in a 0, hence the fruit isn't inserted.

First Apple goes in as the tree is empty. The 2nd & 3rd Apple result in false in all the if conditions, thus returning the final 0. Put a System.out.println() before the final return to confirm.

If you want to sort the fruits first by name, then by weight & then finally by price, here's a more compact way doing it:

@Override
public int compare(Fruits f1, Fruits f2) {
    if (f1.name.equals(f2.name)) {
        if (f1.weight < f2.weight) {
            return -1;
        } else if (f1.weight > f2.weight) {
            return 1;
        } else {
            if (f1.price < f2.price) {
                return -1;
            } else if (f1.price > f2.price) {
                return 1;
            } else {
                return 0;
            }
        }
    } else {
        return f1.name.compareTo(f2.name);
    }
}

edited Jun 22, 2016 at 18:26

answered Jun 22, 2016 at 18:14

Vineet

89710 silver badges27 bronze badges

2 Comments

GAK Over a year ago

Thanks. I believe thats where the problem lies. But i don't understand why the conditions are met in my compare method. Please take a look if you find some time. Thanks.

Vineet Over a year ago

None of your conditions are met. Place a System.out.println(8) after the 7th one, after the complete if block to view this. In every if condition, you have at least one of either name,weight or price being equal/==, whereas none of the three apples have the same weight or price.

Community · Accepted Answer · 2017-05-23 12:33:10Z

TreeSet, when used with a Comparator, the elements' equality is decided by the compare method of the Comparator, otherwise would use the compareTo method of its element since they are required to implement the Comparable interface. The hashcode and equals methods will only be used by the Set interface itself (such as method contains uses equals method to check if the elements are presented). And hashcode is not something that a TreeSet to use while it is used by HashSet which is totally another way to implement Set interface. Thus, in your code, since the compare method you've overridden of the Comparator treats these elements equal, so they cannot be inserted for multiple times. One guideline that the Java Tutorial points out is, the compare method should comply with the equals methods, which is, the elements should be treated equal in the compare method if and only if the equals method do.

And in your equals method, you did use this.weight == f.price to compare two fruits, which I don't think is what you intended to do. This makes your equals methods not consistent with the compare method.

For your reference, see Java Object Ordering tutorial, and as well as a question I asked two days ago.

Jesper · Accepted Answer · 2016-06-22 17:59:48Z

0

You have an error in your equals method in class Fruits:

if(this.name.equals(f.name) && this.price==f.price && this.weight == f.price)

should have been:

if(this.name.equals(f.name) && this.price==f.price && this.weight == f.weight)

(note the last part).

answered Jun 22, 2016 at 17:59

Jesper

208k47 gold badges325 silver badges361 bronze badges

2 Comments

GAK Over a year ago

I changed the typo but still the behavior doesnt change. I believe I need to re look my compare in the comparator class. Thanks.

JackDaniels Over a year ago

TreeSet and TreeMap does not use equals and hashCode methods anyway. They will be used for HashSet and HashMap. For this question, they are irrelevant. Look at my answer for the correct fix. The fix is in the comparator.

Collectives™ on Stack Overflow

Java TreeSet not adding object

4 Answers 4

Comments

2 Comments

Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

2 Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related