• Post Reply Bookmark Topic Watch Topic
  • New Topic

Correct implementation of equals() and hashcode() methods  RSS feed

 
Ankush Seth
Ranch Hand
Posts: 88
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello all, i am here to ask you weather i have implemented the contract correctly or not.
i have gone through the contract and implemented this-





Pardon me if its not a proper place to ask this question.
 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16060
88
Android IntelliJ IDE Java Scala Spring
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No, you did not implement the equals() and hashCode() methods correctly.

Your equals() method only compares on quizQuestion, while your hashCode() method only looks at questionNumber and answer.

The contract for equals() and hashCode() is this: When two objects A and B are equal (so that A.equals(B) returns true), then A.hashCode() must be the same as B.hashCode().

NOTE that this works only in one direction: it does not mean that if A.hashCode() == B.hashCode(), A.equals(B) always has to return true.

With your implementation, it's possible to violate the contract, by having two Question objects with the same value in quizQuestion, but different values in questionNumber and/or answer.

 
Paweł Baczyński
Bartender
Posts: 2087
44
Firefox Browser IntelliJ IDE Java Linux Spring
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Also, your equals() method can possibly throw NullPointerException if this.getQuizText() returns null.
The equals() method can never throw any exception.

If you accept null quiz text, take it into account in your equals().
If you do not accept null quiz text, prevent null value in the setter method.

Also:The second part of this condition is redundant. If other is null, other instanceof Question will be false.


Also:This kind of array declaration, while syntactically correct, is discouraged as per convention.
Preferred one would be:

Also, you never assign anything to options. So, every time you call getOption() or setOption() you will suffer from NullPointerException as options will always be null (default value for member reference (non-primitive) variable).
 
Ankush Seth
Ranch Hand
Posts: 88
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jesper de Jong wrote:No, you did not implement the equals() and hashCode() methods correctly.


Thank you for explaining me. Then what should i do to get it right. Please tell me
 
Ulf Dittmer
Rancher
Posts: 42972
73
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
This should also be of interest: http://www.artima.com/lejava/articles/equality.html
 
Ankush Seth
Ranch Hand
Posts: 88
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Above link is fabulous!!! Thank you very much...)
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ankush Seth wrote:Hello all, i am here to ask you weather i have implemented the contract correctly or not.

First: nice to see that you're using instanceof. Well done.

There is a very good template, described in this book, that's well worth remembering:

1. Check if the "other" object is the same object as this one (ie, if they are '=='). If so, return true. This is one of the very rare cases when using '==' with objects is correct.

2. Check if the "other" object is the same type as this one (ie, your instanceof check). If NOT, return false. Note that, as was already pointed out, this also eliminates the situation where "other" is null.

3. If (and only if) this class has a superclass other than Object, call its equals() method, passing the same "other" object; and if it returns false, then return false - ie:
  if (!super.equals(other)) return false;
(This is the part that most people argue about; but I think it's covered in the link that Ulf gave you)

4. Assign the "other" object to a local variable with the same type as this one. You will have to cast it, but the cast is guaranteed to work because of the check in Step 2. Personally, I like to call the variable 'that', so you have 'this' and 'that'.

5. Finally, compare 'this' and 'that' for equality by comparing fields defined in this class (and ONLY this class).

Alternatively, you could try this idea. Make sure you test it well though,

HIH

Winston
 
Ankush Seth
Ranch Hand
Posts: 88
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you Winston Gutkowski for your overwhelming response. I am glad to have opinion from you!!
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ankush Seth wrote:Thank you Winston Gutkowski for your overwhelming response. I am glad to have opinion from you!!

You're most welcome. Don't forget to look at Ulf's link though - it's one of the best articles you'll find about "equality".

Winston
 
Ankush Seth
Ranch Hand
Posts: 88
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yes i am going through it. It is really very nice !
 
Campbell Ritchie
Marshal
Posts: 56592
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You have already been given two references about the equals() method. Please search for “Angelika Langer Java equals hashCode” and you will find another useful article there. It is worth reading all three since they do not quite cover the same material.
 
Ankush Seth
Ranch Hand
Posts: 88
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yes i will definitely look for this article as well. Thanks a lot!
 
Javin Paul
Ranch Hand
Posts: 297
Eclipse IDE Firefox Browser Linux
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Just to add, if you happen to implement Comparable also, make sure your compareTo() method is compatible with equals() i.e. for same object equals() should return true and compareTo() must return zero.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Javin Paul wrote:i.e. for same object equals() should return true and compareTo() must return zero.

@Ankush: And just to be clear, it should return true when, and ONLY when, compareTo() returns 0 (or vice-versa) - ie, they should be synonymous.

Good point though.

Winston
 
Paweł Baczyński
Bartender
Posts: 2087
44
Firefox Browser IntelliJ IDE Java Linux Spring
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Javin Paul wrote:Just to add, if you happen to implement Comparable also, make sure your compareTo() method is compatible with equals() i.e. for same object equals() should return true and compareTo() must return zero.

No, that is not quite true. It should return zero...
javadoc wrote:It is strongly recommended (though not required) that natural orderings be consistent with equals. This is so because sorted sets (and sorted maps) without explicit comparators behave "strangely" when they are used with elements (or keys) whose natural ordering is inconsistent with equals. In particular, such a sorted set (or sorted map) violates the general contract for set (or map), which is defined in terms of the equals method.

There is one class in Java API that does not have natural ordering consistent with equals. It's BigDecimal. Be careful with it.
 
Javin Paul
Ranch Hand
Posts: 297
Eclipse IDE Firefox Browser Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Agree, It's not mandatory and BigDecimal is a good example, why you should follow this rule.
 
Campbell Ritchie
Marshal
Posts: 56592
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Does everybody here know why BigDecimal has compareTo not consistent with equals?
 
Rob Spoor
Sheriff
Posts: 21135
87
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Because 0.0 != 0.00, but you can't say which one of the two is larger. 0.0 is any number between -0.05 and 0.05 and 0.00 is any number between -0.005 and 0.005. So in some cases 0.0 can be larger than 0.00, and in other cases it can be smaller. Taking the middle way isn't that unreasonable.
 
Campbell Ritchie
Marshal
Posts: 56592
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Rob Spoor wrote: . . . 0.0 != 0.00, . . .
I agree with you about that; I am sure there are people who would take the opposite view, however.
 
Rob Spoor
Sheriff
Posts: 21135
87
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It depends on how you look at it. Mathematically speaking they are equal, but my high school science classes taught me that any decimal is significant due to rounding. With BigDecimal I prefer the scientific approach - there is a reason for those extra zeros.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Rob Spoor wrote:With BigDecimal I prefer the scientific approach - there is a reason for those extra zeros.

OK, here's my counter-argument:

1. BigDecimal is a value class; therefore equals() should be a value comparison.

2. It causes LOTS of confusion.

3. The result can't be converted to an order that accurately reflects the "spread" of values; therefore it's impossible to implement a compareTo() method that's "consistent with" equals() - so, unsurprisingly, BigDecimal's isn't.

4. The designers could have easily added an equivalentTo() method that takes scale() into account.

My suspicion is that they chose that form because a value-based hashcode is much more difficult (and time-consuming) to calculate. But since BigDecimals are immutable, you could easily cache the value; so even if I'm right, I think it was a poor decision.

Winston
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!