Same value of treeset is allowed because of logic error

Question

Hi I everyone I'm now learning java collection and has this assignment. Something is wrong with my logic would anyone please tell me where did I go wrong? Here's my code.

This is Family class where I instantiate the treeSet of mother

import java.util.Comparator;
import java.util.Set;
import java.util.TreeSet;
public class Family {
    Mother mom = new Mother("0", 0, 0);

    Set<Mother> Mothers = new TreeSet<Mother>(new Comparator<Mother>() {
        public int compare(Mother a, Mother b) {
            if(a.getName().compareTo(b.getName()) == 0) return 0;

            if(a.getMonth() == b.getMonth()){
                if(a.getDay() > b.getDay()) return 1;

                else if(a.getDay() < b.getDay()) return -1;
            }
            if(a.getMonth() > b.getMonth()) return 1;

            if(a.getMonth() < b.getMonth()) return -1;

            return a.getName().compareTo(b.getName());
        }
    });

    public Family(){}

    public boolean addMother(java.lang.String name, int day, int month)
    {
        Mother a = new Mother(name, day, month);
        return Nieces.add(a);
    }
}

This is Main class where I assign the TreeSet of Mother.

public class Main {
    public static void main(String[] args) {
       Family myFam = new Family();
       myFam.addMother("Tokyo", 1, 3);
       myFam.addMother("Shaline", 1, 1);
       myFam.addMother("Betty", 2, 3);
       myFam.addMother("Amy", 1, 1);
       myFam.aaddMother("Shaline", 4, 3);
       System.out.println(myFam.Mothers);
    }
}

So I want to sort it by the date and month.The output I got is : [Amy, Shaline, Tokyo, Betty, Shaline] which true because it sorted based the date and the month but I expected that it would be only one Shaline since it is a TreeSet. What can I do so the output can be : [Amy, Shaline, Tokyo, Betty]. I also already tried like if(a.getName().equals(b.getName())) return 0;

But it still not working.Thank you for the help!

Answer 1

Your comparator is inconsistent. See the contract of comparator:

The implementor must also ensure that the relation is transitive:
((compare(x, y)>0) && (compare(y, z)>0)) implies compare(x, z)>0.

This is violated by your comparator, as when x is "Shaline", 1, 1 and y is "Betty", 2, 3 and z is "Shaline", 4, 3, it is reporting x < y and y < z using the month or day, so the transitivity rule would require that x < z, but your comparator reports x == z by returning zero due to the equal names.

The TreeSet relies on the transitivity, as it allows an efficient lookup, by not comparing every element with the new element on insertion, but using the order to navigate to the right place in the tree.

You can add a linear search, like in this answer, but that looses the efficiency of the Set as it now compares every contained element.

Instead, I recommend something like:

Set<String> names = new HashSet<>();
Set<Mother> mothers = new TreeSet<>(Comparator.comparingInt(Mother::getMonth)
        .thenComparingInt(Mother::getDay).thenComparing(Mother::getName));

public Family(){}

public boolean addMother(java.lang.String name, int day, int month)
{
    return names.add(name) && mothers.add(new Mother(name, day, month));
}

This simplifies and fixes the comparator, which now consistently uses all properties of the Mother object and adds an efficient pretest.

Answer 2

On adding, just check whether the new Mother is unique:

    public boolean addMother(java.lang.String name, int day, int month)
    {   
        for (Mother m: Mothers) {
            if (m.getName().equals(name)) {
                return false; // suppose the result should be false in case of not adding
            }
        }

        Mother a = new Mother(name, day, month);
        return Nieces.add(a);
    }