• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

overiding equals() method

 
Ranch Hand
Posts: 541
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I have never overridden fancy equals() or hashCode() methods.

I never used Set before either. As I am using Set now, I need to use equals() method. I used IDE to generate the method, I selected 3 different fields as input to generate this method. It did generate the method but it checks the equality with individual fields not for unique combination of all three fields.

I need unique record of that combination added to the Set.

I can modify the IDE generated equals method according to my need (without ending the world), right? Just checking :-)
 
Bartender
Posts: 4568
9
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Yes, rewriting IDE generated code is fine. I'm surprised the IDE didn't do what you needed, though. Are you sure it's not right? Feel free to post the code here for a second opinion.
 
Saurabh Pillai
Ranch Hand
Posts: 541
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
This is what I have from IDE generated method (project specific information renamed) . I want uniqueness in combination of all 3 fields.

 
Marshal
Posts: 28175
95
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Well, there's a copy-paste-forget-to-edit error at line 14, but apart from that it returns false if any of the three fields are different between the two objects. Is that not what you wanted?
 
Ranch Hand
Posts: 808
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Line 14 erroneously reads when it should read

Edit: d'oh! too slow!
 
Saurabh Pillai
Ranch Hand
Posts: 541
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Paul Clapham wrote:Well, there's a copy-paste-forget-to-edit error at line 14, but apart from that it returns false if any of the three fields are different between the two objects. Is that not what you wanted?



Yes, I forgot to edit fully. Sorry about that. Now I have fixed it.

Consider this,

1) field1 = 1, field2 = 2, field3=3 It would be added to Set.
2) field1 = 1, field2=2, field3=2 This record would be discarded according for IDE generated equals() method because field1 id duplicate. but for my requirement it should add to Set because the combination is unique.

I want validation only for UNIQUE combination.

I guess code should be modified to something like,

 
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Saurabh Pillai wrote:

Paul Clapham wrote:Well, there's a copy-paste-forget-to-edit error at line 14, but apart from that it returns false if any of the three fields are different between the two objects. Is that not what you wanted?



Yes, I forgot to edit fully. Sorry about that. Now I have fixed it.

Consider this,

1) field1 = 1, field2 = 2, field3=3 It would be added to Set.
2) field1 = 1, field2=2, field3=2 This record would be discarded according for IDE generated equals() method because field1 id duplicate.



I highly doubt that. Unless you've changed it, the eqauls() that the IDE generates will only return true if all fields are equal. Please post the IDE-generated equals(), without any modifications.
 
Paul Clapham
Marshal
Posts: 28175
95
Eclipse IDE Firefox Browser MySQL Database
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
No. Your equals() method was perfectly good the way it was. The only problem was that you were thinking of the rules for adding an element to a Set, instead of just thinking about how to tell if two objects were equal and you got yourself all mixed up.

So: the equals() method returns false when you compare (1, 2, 3) to (1, 2, 2) because the third field is different. So if you try to add those two objects to a Set, they both get added because they are not equal to each other. The code you posted returns true only if all three fields are the same.

You seemed to think that the second one would be "discarded" because of... well, I can't tell why. I can tell you that the Set implementations don't look at the code inside the equals() method and try to interpret it. They just look at the result of the equals() method and act accordingly.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Saurabh Pillai wrote:This is what I have from IDE generated method (project specific information renamed) . I want uniqueness in combination of all 3 fields.



This method will do what you want. (At least I think it's what you want.) It will return true only if all the fields are equal. So if you have


Then both objects will be added to the Set, because they are not equal.

Note than whenever you override equals(), you should also override hashCode().
 
Marshal
Posts: 79151
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I wouldn’t write an equals() method with all those return statements myself.

But there is an excuse for that style. This is not code you have written, but code automatically generated. Automatically generated code is exempt from style guidelines.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Campbell Ritchie wrote:I wouldn’t write an equals() method with all those return statements myself.



Do you use a "result" variable? Chained ternary operators? Other?
 
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Saurabh Pillai wrote:I can modify the IDE generated equals method according to my need (without ending the world), right? Just checking :-)


Yes. In fact I'd recommend it, because that code is
(a) awful, and
(b) (possibly) not what you want.

Unfortunately, there's a lot to know about equals(), and a lot of ways to write them (right and wrong).
The line your IDE has taken is to use an actual class comparison as part of the "equality decision", which I really don't like (and I expect disagreement from my colleagues here), even though it is generally regarded as the "safest" route.

Why don't I like it? Because it violates LSP; and furthermore, it usually violates it without documenting the fact, and the errors it produces are subtle (and specifically target collections).

My preferred pattern, if you're still awake, is:It's fast (quite a bit faster than using a reflective method like getClass()); and it eliminates all that poxy == null/!= null stuff (instanceof returns false if the right-hand argument is null).

Furthermore, it will work for any subclass that doesn't require any additional value checks - and it's there that you usually run into problems. I leave it to you to read all about them.

The only other tip I can give you is to use equals() for all reference field checks that are part of your method, and '==' for all primitives; and forget all that null nonsense.

HIH

Winston
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Saurabh Pillai wrote:I guess code should be modified to something like,...


BTW, I broke up that huge line in your code block (my first act as newly promoted mini-God); I hope you don't mind. They screw up the windowing here.

Winston
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:

Saurabh Pillai wrote:I can modify the IDE generated equals method according to my need (without ending the world), right? Just checking :-)


Yes. In fact I'd recommend it, because that code is
(a) awful, and



I wouldn't change it just because it's ugly. I agree with CR's stance that generated code is exempt from coding conventions.

Unfortunately, there's a lot to know about equals(), and a lot of ways to write them (right and wrong).
The line your IDE has taken is to use an actual class comparison as part of the "equality decision", which I really don't like (and I expect disagreement from my colleagues here), even though it is generally regarded as the "safest" route.



It really depends on your requirements and design. If you want a Child to be able to be equal to a Parent, then of course you have to use instanceof. But if not, then use getClass(). The key thing to remember is that once you use instanceof X somewhere in a hierarchy, then everything below X must also use instanceof X, lest ye violate symmetry.

Having said all that, I can't remember the last time I used getClass(). I know I've used it, and I thought I had a good reason to do so at the time, but it was long ago and it may have been a bad decision.

It's fast (quite a bit faster than using a reflective method like getClass());



Can you back that up? I'd expect getClass() to be faster than, or at least as fast as, instanceof. getClass() gives us the object's "leaf type", which I would expect is available pretty directly. Depending on how that information is stored, instanceof may have to walk up the tree starting with the leaf type--that is, may have to effectively getClass() anyway. At the very least, if all a class's types are stored directly with that class, without having to probe its ancestor types, getClass() and instanceof should perform about the same.

Really, instanceof is no less reflective than getClass(), and may be more so.

The only other tip I can give you is to use equals() for all reference field checks that are part of your method, and '==' for all primitives; and forget all that null nonsense.



Except that we still have to check each reference field for null, unless our preconditions say that given field cannot be null.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Jeff Verdegan wrote:Can you back that up? I'd expect getClass() to be faster than, or at least as fast as, instanceof...
Really, instanceof is no less reflective than getClass(), and may be more so.


From the evidence of testing on my own machine - absolutely. instanceof takes, in general, a handful of nanoseconds (usually 3 or 4 for a basic class, add a couple for hierarchies; although I have to admit, I've never checked it with an enormous one); getClass() usually takes around 30.

Now, why that is, I really have no idea; although it makes me suspect that the JVM might be caching certain runtime info for the job.

@Saurabh: That said, time (especially miniscule amounts like that) should never be a determining factor when choosing what's right; I only included it to bolster my argument (which, as you can see, is not shared by all ).

Winston
 
dennis deems
Ranch Hand
Posts: 808
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:

Saurabh Pillai wrote:I can modify the IDE generated equals method according to my need (without ending the world), right? Just checking :-)


Yes. In fact I'd recommend it, because that code is
(a) awful


It's not awful. You can glance at it and tell at every step what decision is being made and what the outcome will be. Eclipse generates an implementation that is very like what is recommended by Josh Bloch. Note that during generation you can select the fields that are examined; it's not as if you are forced to take all or nothing.

Of course, if your goal is to learn by getting your hands dirty implementing equals and hashCode, then you shouldn't be using a generated method in the first place.

Furthermore, it will work for any subclass that doesn't require any additional value checks - and it's there that you usually run into problems. I leave it to you to read all about them.

This is a very good point which I concede.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dennis Deems wrote:

Winston Gutkowski wrote:

Saurabh Pillai wrote:I can modify the IDE generated equals method according to my need (without ending the world), right? Just checking :-)


Yes. In fact I'd recommend it, because that code is
(a) awful


It's not awful. You can glance at it and tell at every step what decision is being made and what the outcome will be.



Your eyes and/or brain must be better than mine. (Ah, the wonders of youth!) It takes me a deal more than a glance to see what it's doing. Sure, I can tell at a glance roughly what it's intended to do (assuming I also read the method name), but, as for what it actually does, seriously, ICK!

Eclipse generates an implementation that is very like what is recommended by Josh Bloch.



There are lots of styles that produce the same functionality. I respect Bloch's technical acumen, but when it comes to how the code reads, I don't grant him any particular authority.

Of course, if your goal is to learn by getting your hands dirty implementing equals and hashCode, then you shouldn't be using a generated method in the first place.



Amen.
 
Campbell Ritchie
Marshal
Posts: 79151
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Jeff Verdegan wrote: . . . Do you use a "result" variable? Chained ternary operators? . . .

Both, at different times.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dennis Deems wrote:It's not awful. You can glance at it and tell at every step what decision is being made and what the outcome will be. Eclipse generates an implementation that is very like what is recommended by Josh Bloch.


No it doesn't. In fact it implements something specifically NOT recommended by him (at least in Effective Java; and expanded on in ed.2) - using getClass().

Which, oddly enough, is why I brought it up.

Winston

[Edit] I will concede your initial point, which was that the IDE code (although verbose to me) is clear.
 
Saurabh Pillai
Ranch Hand
Posts: 541
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thank you all for all the brain storming. Winston, there is an option to use instanceof operator instead of getClass().

Paul Clapham wrote:The only problem was that you were thinking of the rules for adding an element to a Set, instead of just thinking about how to tell if two objects were equal and you got yourself all mixed up.



That is correct :-)


You need equals() to return false in order to add the element.
HashSet.add() returns true if it adds the element.
Moreover, at every steps of that equals() method there is a return statement with true/false. I like only one exit point in method.
Also, I prefer this,

written as wherever it reduces the confusion for me.


So it all got mixed up. But today with fresh mind it all became clear.

Thank you all again.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Saurabh Pillai wrote:
Also, I prefer this,

written as wherever it reduces the confusion for me.



Those two are *NOT* equivalent. For any object with more than one field, they will behave very differently.
 
Don't get me started about those stupid light bulbs.
reply
    Bookmark Topic Watch Topic
  • New Topic