File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Beginning Java and the fly likes overiding equals() method Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Murach's Java Servlets and JSP this week in the Servlets forum!
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "overiding equals() method" Watch "overiding equals() method" New topic
Author

overiding equals() method

Saurabh Pillai
Ranch Hand

Joined: Sep 12, 2008
Posts: 506
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 :-)
Matthew Brown
Bartender

Joined: Apr 06, 2010
Posts: 4342
    
    7

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

Joined: Sep 12, 2008
Posts: 506
This is what I have from IDE generated method (project specific information renamed) . I want uniqueness in combination of all 3 fields.

Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

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?
dennis deems
Ranch Hand

Joined: Mar 12, 2011
Posts: 808
Line 14 erroneously reads when it should read

Edit: d'oh! too slow!
Saurabh Pillai
Ranch Hand

Joined: Sep 12, 2008
Posts: 506
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,

Jeff Verdegan
Bartender

Joined: Jan 03, 2004
Posts: 6109
    
    6

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
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

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

Joined: Jan 03, 2004
Posts: 6109
    
    6

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().
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 37936
    
  22
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

Joined: Jan 03, 2004
Posts: 6109
    
    6

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?
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7503
    
  18

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


Isn't it funny how there's always time and money enough to do it WRONG?
Articles by Winston can be found here
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7503
    
  18

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

Joined: Jan 03, 2004
Posts: 6109
    
    6

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

Joined: Mar 17, 2011
Posts: 7503
    
  18

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

Joined: Mar 12, 2011
Posts: 808
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

Joined: Jan 03, 2004
Posts: 6109
    
    6

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
Sheriff

Joined: Oct 13, 2005
Posts: 37936
    
  22
Jeff Verdegan wrote: . . . Do you use a "result" variable? Chained ternary operators? . . .
Both, at different times.
Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 7503
    
  18

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

Joined: Sep 12, 2008
Posts: 506
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

Joined: Jan 03, 2004
Posts: 6109
    
    6

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.
 
subject: overiding equals() method
 
Similar Threads
Problem with hashcode etc
Comparing two Java Objects
Authenticating in Spring Security with password and username in url
Comparators
Do you include the references in the equals