custom equals() method does not work properly

Question

I've implemented the Apriori algorithm. it works pretty well, but I ran into a strange problem: I've defined a Rule class to maintain the generated rules.

Here it is:

public class Rule 
{
    private Set<Integer> left;
    private Set<Integer> right;
    private LookupArtist lookupArtist;

    public Rule(LookupArtist lookupArtist){
        left = new HashSet<>();
        right = new HashSet<>();
        this.lookupArtist = lookupArtist;
    }

    @Override
    public boolean equals(Object another){
        Rule rule = (Rule) another;

        if(this.left.equals(rule.getLeft()) && this.right.equals(rule.getRight()))
            return true;
        else
            return false;
    }

    @Override
    public String toString(){
        /* print the object */
    }

    public void addToLeft(Integer toAdd){
        left.add(toAdd);
    }

    public void addToRight(Integer toAdd){
        right.add(toAdd);
    }

    public Set<Integer> getLeft(){
        return left;
    }

    public Set<Integer> getRight(){
        return right;
    }

}

I also implemented the equals() method in a different way just to try:

@Override
    public boolean equals(Object another){
        Rule rule = (Rule) another;
        boolean flag = true;
        for(Integer artist : left){
            if(flag)
            if(!rule.left.contains(artist))
                flag=false;
        }

        if(flag)
            for(Integer artist : right){
                if(flag)
                if(!rule.right.contains(artist))
                    flag=false;
            }
        return flag;

    }

The LookupArtist object is used to map the integers to some Strings.

The problem is that when I print out the rules I found that some rules appear two times. I also found in debug mode some replicated rules, so it isn't be a print problem. The rules are saved in a map like this:

static Map<Rule, Float> rules;

.
.
.

Rule rule = new Rule(lookupArtist);
for(int j=0;j<i;j++){
    rule.addToLeft(a[j]);
}
for(int j=i;j<a.length;j++){
    rule.addToRight(a[j]);
}
if(!rules.containsKey(rule)){
    rules.put(rule, getRuleConfidence(rule));
}

Any idea where the problem can be?

Eran · Answer 1 · 2014-11-30T10:42:37.313

When using a HashSet for storing objects of a class that has a custom equals implementation, you must have a matching custom implementation for hashCode.

If two objects are equal (according to the custom equals implementation), they must have the samehashCode. In the code you posted, I don't see an overriding ofhashCodein theRule` class.

When you add an instance to the HashSet, hashCode method is used to determine the index in the hash table in which the instance will be stored. Then, the linked list of instances stored in this index is iterated to see if the instance is already there. When iterating over that list, equals is used. If two objects that are equal are mapped by hashCode to different indices in the HashSet, the duplication won't be detected, since they would be stored in separate linked lists.

This is stated in the Javadoc of equals :

 * Note that it is generally necessary to override the <tt>hashCode</tt>
 * method whenever this method is overridden, so as to maintain the
 * general contract for the <tt>hashCode</tt> method, which states
 * that equal objects must have equal hash codes.

And in the Javadoc of hashCode :

 * <li>If two objects are equal according to the <tt>equals(Object)</tt>
 *     method, then calling the <code>hashCode</code> method on each of 
 *     the two objects must produce the same integer result.

score 1 · Answer 2 · answered Nov 30 '14 at 10:37

1

And where is your hashCode() method? It is also very important :)

answered Nov 30 '14 at 10:37

m0tylan0ga

86
3

score 1 · Accepted Answer · edited May 23 '17 at 12:12

You should always override hashCode when you override equals and vice versa.

Add something like this to your Rule class:

@Override
public int hashCode() {
    return left.hashCode()
         ^ right.hashCode()
         ^ lookupArtist.hashCode();
}

Here is a good answer explaining why it's important to override both.

Also, your equals method can be written as

@Override
public boolean equals(Object another){
    Rule rule = (Rule) another;
    return left.equals(rule.left)
        && right.equals(rule.right)
        && lookupArtist.equals(rule.lookupArtist);
}

A final remark: Your other attempt at the equals-implementation is not symmetrical, i.e. it's not the case that rule1.equals(rule2) if and only if rule2.equals(rule1). That's a violation of the contract of equals.

custom equals() method does not work properly

3 Answers3