This discussion is archived
1 2 Previous Next 24 Replies Latest reply: Jul 24, 2009 10:36 AM by 3004 RSS

Am I overriding hashCode(), equals(), and compareTo() correctly?

843789 Newbie
Currently Being Moderated
Hello,

I am creating the below code for "public class User implements Comparable<User>". Is okay to override hashCode in this manner and then utilize it in the other two methods? The class User has other attributes (everything String) that I did not include in hashCode(), should I do this to be "thorough" or would it be excessive? Are there any other overall issues with code like this?

Thanks.
public int hashCode(){
          return _firstName.hashCode()+_lastName.hashCode()+ _email.hashCode();
     }

public boolean equals(Object other){
          boolean greatSuccess = false;
          if((other == null) || (!(other instanceof User))){
               return greatSuccess;
          }          
          User that = (User) other;          
          if (this.hashCode() == that.hashCode()){
               greatSuccess = true;
          }          
          return greatSuccess;
     }

public int compareTo(User that) {
          int comparison = 0;
          if(this.hashCode() < that.hashCode()){
               comparison = -1;
          }else if(this.hashCode() > that.hashCode()){
               comparison = 1;
          }
          return comparison;
     }
  • 1. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    843789 Newbie
    Currently Being Moderated
    Is okay to override hashCode in this manner and then utilize it in the other two methods?
    No.
    Are there any other overall issues with code like this?
    Yes. It is possible for two unequal string objects to produce the same hashCode. In your code, it would be possible have two users, with different first names, last names, and email addresses that could be considered "equal" (i.e., equals() returns true).

    It might help to review the contracts for equals() and hashCode() in the java.lang.Object API javadocs.

    ~
  • 2. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    798635 Newbie
    Currently Being Moderated
    If you are using String's hashCode() method to generate your own hashCode() (i.e. firstName, lastName, _email are Strings), I would say your other methods would fail quite often.  Unless you have a perfect hashCode() method (nearly impossible other than for trivial data types), you can't define your entire equals() method based off the hash code values.  You may use it to do a speedy fail, however.  The compareTo() is making way too much assumptions, but, again, I don't know what the member field types are.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   
  • 3. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    843789 Newbie
    Currently Being Moderated
    I see that I've erred in adding together the int values in hashCode(). Would this be remedied by concatenating first as Strings? Or am I just using overall bad technique here?

    Thanks
    public int hashCode(){          
              String temp = _firstName + _lastName + _email;
              return temp.hashCode();
    }
  • 4. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    843789 Newbie
    Currently Being Moderated
    chaddy_b wrote:
    Or am I just using overall bad technique here?
    Yeah, sorry. If I may be so bold, allow me to suggest something like the following:
    public int hashCode(){
        int result = _lastName.hashCode();
        result = 31 * result + _firstName.hashCode();
        result = 31 * result + _email.hashCode();
        return result;
    }
    
    public boolean equals(Object other) {
        if (this == other) return true;
        if (!(other instanceof User)) return false;
        User that = (User) other;
        if (!this._lastName.equals(that._lastName)) return false;
        if (!this._lastName.equals(that._firstName)) return false;
        if (!this._lastName.equals(that._email)) return false;
        return true;
    }
    
    public int compareTo(User that) {
        int lastNameDiff = this._lastName.compareTo(that._lastName);
        if (lastNameDiff != 0) return lastNameDiff;
        
        int firstNameDiff = this._firstName.compareTo(that._firstName);
        if (firstNameDiff != 0) return firstNameDiff;
        
        return this._email.compareTo(that._email);
    }
    You'll want to account for the other fields in your User class as well. You may also want to look into using the HashCodeBuilder and EqualsBuilder classes from the Apache Commons Lang library. I also highly recommend having a copy of the following book at your desk:

    [Effective Java - Joshua Bloch|http://java.sun.com/docs/books/effective/]

    Good luck!

    ~
  • 5. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    3004 Newbie
    Currently Being Moderated
    You'll probably want to read this: [http://java.sun.com/developer/Books/effectivejava/Chapter3.pdf]
  • 6. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    798635 Newbie
    Currently Being Moderated
    I am a firm believer that equals(null) and compareTo(null) should not bomb.
  • 7. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    3004 Newbie
    Currently Being Moderated
    raychen wrote:
    I am a firm believer that equals(null) and compareTo(null) should not bomb.
    IIRC, by contract, equals(null) is not allowed to, but compareTo(null) is.
  • 8. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    843789 Newbie
    Currently Being Moderated
     public boolean equals(Object other) {
        if (this == other) return true;
        if (!(other instanceof User)) return false;
    Common bug alert! This makes the requirement "a.equals(b) == b.equals(a)" fail when subclasses with their own equals() are involved. Instead of instanceof use getClass():
     public boolean equals(Object other) {
        if (this == other) return true;
        if (other == null ) return false;
        if (this.getClass() != other.getClass()) return false;
  • 9. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    3004 Newbie
    Currently Being Moderated
    sjasja wrote:
    public boolean equals(Object other) {
    if (this == other) return true;
    if (!(other instanceof User)) return false;
    Common bug alert! This makes the requirement "a.equals(b) == b.equals(a)" fail when subclasses with their own equals() are involved. Instead of instanceof use getClass():
    public boolean equals(Object other) {
    if (this == other) return true;
    if (other == null ) return false;
    if (this.getClass() != other.getClass()) return false;
    It's a bit more complicated than that.

    If you use instanceof, then every descendant of that class must use instanceof. Sometimes instanceof is correct. For instance, any List, Set, or Map is equal to any other List, Set, or Map, respectively, regardless of the implementation classes, as long as the contract for the interface in question is met. This means that every List, Set, and Map implementation must use isntanceof.

    Edited by: jverd on Jul 24, 2009 9:44 AM
  • 10. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    798635 Newbie
    Currently Being Moderated
    This is slightly off topic, but which is faster (when they are equivalent) generally speaking?
    a instaceof b
    or
    a.getClass() == b.getClass()
    My gut feeling says getClass(), as instanceof uses reflection.
  • 11. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    3004 Newbie
    Currently Being Moderated
    raychen wrote:
    This is slightly off topic, but which is faster (when they are equivalent) generally speaking?
    a instaceof b
    or
    a.getClass() == b.getClass()
    My gut feeling says getClass(), as instanceof uses reflection.
    I would think getClass, since it only has to call two fast methods and to an == comparison, whereas instanceof could potentially have to walk a big inheritance tree.

    Of course, as always, the answer to a "which is faster" question is to test it for yourself.
  • 12. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    843789 Newbie
    Currently Being Moderated
    raychen wrote:
    This is slightly off topic, but which is faster (when they are equivalent) generally speaking?
    a instaceof b
    or
    a.getClass() == b.getClass()
    My gut feeling says getClass(), as instanceof uses reflection.
    They are not equivalent.
  • 13. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    3004 Newbie
    Currently Being Moderated
    sabre150 wrote:
    raychen wrote:
    This is slightly off topic, but which is faster (when they are equivalent) generally speaking?
    a instaceof b
    or
    a.getClass() == b.getClass()
    My gut feeling says getClass(), as instanceof uses reflection.
    They are not equivalent.
    D'OH! Of course. Good point. (Way to ignore the important stuff, Jeff.)

    EDIT: Although he did say "when they are equivalent," which I guess means, "when they would produce the same result," which is when both are true and when both are false.

    Edited by: jverd on Jul 24, 2009 9:48 AM

    EDIT again: Of course, that's pointless anyway, since there's no case when you can choose between them. Only one will be correct in any given situation.

    Edited by: jverd on Jul 24, 2009 9:50 AM
  • 14. Re: Am I overriding hashCode(), equals(), and compareTo() correctly?
    843789 Newbie
    Currently Being Moderated
    Sometimes instanceof is correct. For instance, any List, Set, or Map is equal to any other List, Set, or Map, respectively, regardless of the implementation classes, as long as the contract for the interface in question is met.
    Deja vu (that's when they change things?) http://forums.sun.com/thread.jspa?threadID=626593

    Collections have their own special rules on equals(). In your typical garden variety class I'd use getClass() by default.
1 2 Previous Next