HashSet adds two objects which returns true for equals() and has same hashcode in Java

The frequencySet() is calculating frequency of each character of a String in an Integer[] wrapped into Counter class which has overriden equals and hashcode. This method is supposed to return only unique frequency in a set but adds both the Counter objects.

As can be seen from the print statement: hashcode() returns equal values and and equals() returns true.

What's wrong?


class Ideone
{
    public static void main (String[] args) throws java.lang.Exception
    {
        // your code goes here
        String[] a = new String[2];
        a[0]="tan";
        a[1]="nat";
        Set<Counter>  s = frequencySet(a);
        System.out.println(s.size()); // prints
        System.out.println(getFreq(a[0]).equals(getFreq(a[1])) + ":" + getFreq(a[0]).hashcode() + ":" + getFreq(a[1]).hashcode() );
    }
public static Set<Counter> frequencySet(String[] strs) {
        Set<Counter> set =  new HashSet<>();
        for(String s: strs){
            Counter counter = getFreq(s);
            set.add(counter);
            //System.out.println(s + " : " + counter.hashcode() + " : " + Arrays.toString(counter.arr) );
        }
        return set;
    }
    
    private static  Counter getFreq(String s){
        Integer[] frequency = new Integer[26];
        for(char c : s.toCharArray()){
            Integer  a =  frequency[c-'a'];
            if(frequency[c-'a']==null){
                frequency[c-'a']=1;
            }
            else{
                a++;
                frequency[c-'a']=a; //++frequency[c-'a'];  
            }
        }
        //System
        return new Counter(frequency);
    }
}
    class Counter{
        Integer[] arr;
        public Counter(Integer[] arr){
            this.arr=arr;
        }
        
        public int hashcode(){
            //return Arrays.deepHashCode((Integer[])arr);
            int hashcode=31;
            for(int i=0;i<arr.length;i++){
                if(arr[i]==null)
                    hashcode+=i;
                else
                    hashcode+=31*arr[i];
            }
            return hashcode;
        }
        
        public boolean equals(Object o){
            //return Arrays.deepEquals(arr,(Integer[])o);
            Counter c = (Counter)o;
            Integer[] other = c.arr;
            for(int i=0;i<26;i++){
                if((arr[i]==null && other[i]!=null) || (arr[i]!=null && other[i]==null)){
                    return false;
                }
                else if(arr[i]!=null && other[i]!=null && arr[i]!=other[i]){
                    return false;
                }
            }
            return true;
        }
    }

Output:

    2
    true:417:417

<script src="https://ideone.com/e.js/s2LcBw" type="text/javascript" ></script>

Solution

Always use @Override. You've implemented method public int hashcode(). That's not what you think it is. You want public int hashCode(). Note the capital c. add @Override annotations and the compiler would have errored (try it first: @Override public int hashcode()..., then compile it, notice the error, fix it by making that hashCode.

Note that your hashCode impl is extremely inefficient and weird. You've heard something about prime numbers but haven't implemented it correctly. The right approach is to multiply the hashcode, as you were writing it, by a prime if you must, not the next element value.

More generally, Arrays.deepHashCode is just fine here. I assume this is what happened:

You wrote your hashcode impl with deepHashCode.
The code didn't work.
You (mistakenly) blamed deepHashCode as the culprit and wrote your own hasher.
It still doesn't work - logical, as that wasn't the problem (writing hashcode with lowercase c is the problem).

Revert your 'fix' - this code with deepHashCode is easier to read and more efficient.