Missing something HashSet duplicates

Question

Ok Apparently I'm missing something here. I cannot seem to get a HashSet to work. I'd imagine it's probably something easy I'm just overlooking.

import testing.Subclass;
import java.util.HashSet;

public class tester{
  public static void main(String[] args) throws Exception{
    HashSet<Subclass> set = new HashSet<Subclass>();
    set.add(new Subclass("007812"));
    set.add(new Subclass("007813"));
    System.out.println("Set size " + set.size());
    set.add(new Subclass("007812"));
    System.out.println("Set size " + set.size());

    for(Subclass sub : set){
      System.out.println(" sub acctNbr " + sub.getAcctNbr());
    }
  }
}

Subclass

public class Subclass implements Comparable<Subclass>{

  public Subclass(String acctNbr) {
    this.acctNbr = acctNbr;
  }
  private String acctNbr;
  public String getAcctNbr(){
    return this.acctNbr;
  }
  public int compareTo(Subclass other){
    return this.getAcctNbr().compareTo(other.getAcctNbr());
  }

  public boolean equals(Subclass other){
    if(other.getAcctNbr().equals(this.getAcctNbr()))
      return true;
    else
      return false;
  }
  public int hashCode(){
    return acctNbr.hashCode();
  }
}

This code outputs

sross@sross-workstation:~/Documents$ javac testing/Subclass.java
sross@sross-workstation:~/Documents$ javac tester.java
sross@sross-workstation:~/Documents$ java tester
Set size 2
Set size 3
 sub acctNbr 007812
 sub acctNbr 007812
 sub acctNbr 007813
sross@sross-workstation:~/Documents$

Answer 1

You need to override equals(Object). Instead of doing this you've implemented an equals method with signature equals(Subclass). Consequently your HashSet is using the default equals(Object) method defined on Object for equality testing.

The default equals(Object) implementation is based on object identity, and hence the set "allows" you to add two Strings that, whilst semantically equal, are not the same object.

Answer 2

You did not correctly override Object.equals().

@Override
public boolean equals(Object other) {
    if ((other == null) || !(other instanceof Subclass)) {
        return false;
    }
    return ((Sublcass) other).getAcctNbr().equals(this.getAcctNbr());
}

The method boolean equals(Subclass other) creates a second method which is not what you intended to do.

Answer 3

First guess, it looks like your equals(Subclass other) ought to be equals(Object other) in order to override the java.lang.Object.equals() method, as you want. Probably the set is calling the underlying equals() implementation.

Answer 4

Your equals method is never called. The signature of equals requires that it take an Object, not some other class (including whatever class happens to be implementing equals).

public boolean equals(Object other) {
    ...
}

Answer 5

Two meta-points:

First, get in the habit of using @Override every time you believe you are overriding a method. That would have caused your example code to fail to compile, leading you to discover the problem.

Second, if you're using an IDE, and it didn't highlight a nice bold warning for you, it is misconfigured! You should fix it!

And if you're not using an IDE -- you really, really should be. As soon as you typed public boolean equals(Subclass other), the text would change color and a warning would be displayed telling you what your likely problem is.

Incidentally, the standard idiom for equals() that I've converged on is this:

@Override public boolean equals(Object object) {
  if (object instanceof Subclass) {
    Subclass that = (Subclass) object;
    return this.anInt == that.anInt
        && this.aString.equals(that.aString); // for example
  }
  return false;
}

In some cases, it is worth prepending an if (object == this) { return true; } but it's really not worthwhile to make a regular habit of it.