• Post Reply Bookmark Topic Watch Topic
  • New Topic

equals compares the wrong things  RSS feed

 
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'm looking at a class with this method

public boolean equals(Object o){
if (o instanceof MyObject) {
MyObject other = (MyObject) o;

return new EqualsBuilder ()
.append(this.getPropA(), other.getPropA())
.append(this.getPropB(), other.getPropB())
.append(this.getPropC(), other.getPropC())
.isEquals();
}

return false;
}

And in another class I'm trying to do this

List duplicateList = (ArrayList) CollectionUtils.intersection(listA, listB);

All the lists are of type MyObject.

I've discoverd that the equals method does not satisfy my requirements. It turns out that I am comparing objects that were instantiated in different ways.

So for the objects to be equal for my purpose I would need this method:


public boolean equals(Object o){
if (o instanceof MyObject) {
MyObject other = (MyObject) o;

return new EqualsBuilder ()
.append(this.getPropX(), other.getPropX())
.append(this.getPropY(), other.getPropX())
.append(this.getPropZ(), other.getPropZ())
.isEquals();
}

return false;
}

I cannot extend the class ( the class is not final, I 'm just not allowed to), and I don't really want to loop through each list.

So can anyone point me to a way to 'pass in' my own equals method?

Thanks
 
Bartender
Posts: 4181
22
IntelliJ IDE Java Python
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
This sounds like a flawed 'equals' method to me. Either the objects are equal or not, it shouldn't depend on how they were made...

Still, you may be able to get around the problem using a SortedSet with a Comparator that makes the comparison you want. The cardinality of the items in the resulting collection would be incorrect (you would end up with just one of each equal object) but you might be able to do more to re-build the correct count.

pseudocode:
 
John Le
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I've looked around the system and it does seem to me that the equals method is incorrectly implemented.....however; I'm not allowed to fix it. The set idea is good for eliminating duplicates, but I need to find the duplicates and report the dups back to the user.

I've decided to just loop through the collection, I was just hoping that there was a 'cooler' way to do it.

Thanks for the reply,
John
 
Steve Luke
Bartender
Posts: 4181
22
IntelliJ IDE Java Python
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by John Le:
I've looked around the system and it does seem to me that the equals method is incorrectly implemented.....however; I'm not allowed to fix it. The set idea is good for eliminating duplicates, but I need to find the duplicates and report the dups back to the user.

I've decided to just loop through the collection, I was just hoping that there was a 'cooler' way to do it.

Thanks for the reply,
John


That is where the cardinality comes in. If you first use the Sets to get the intersection then you can loop through the set, use CollectionUtils.cardinality to determine how many duplicates to report, then add that many duplicates into the final list.

To duplicate the functionality of the CollectionUtils.intersection() method, you would need to check the cardinality of both listA and listB and use the smallest value. Something like:



Or something like that. It should mean you don't have to loop through so many objects, and maybe simplifies things. And of course you could put it all in a method in a utility class all your own that gets called like:

Again, not tested but you get the idea...
 
Steve Luke
Bartender
Posts: 4181
22
IntelliJ IDE Java Python
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Come to think about it, in total you may be looping through your lists a few times using this strategy, there is probably a better strategy...

1) Loop through each object in the list. Create a map with the Object as the key and an integer representing the number of times the object was found in the list as the value.
2) Use the Key Set for the map to generate the intersection as described above
3) Loop through intersection, pull the cardinality out of the Map from step 1 to determine the number of duplicates to add to the list

This probably saves a bunch of looping that must happen in the CollectionUtils.cardinality method.

-- Another reason not to rely on the CollectionUtils.cardinality is that this would also depend on the built - in Equals method, and not your comparator... If you use a SortedMap with this new idea then you shouldn't need to worry about that either.
[ December 19, 2008: Message edited by: Steve Luke ]
 
Steve Luke
Bartender
Posts: 4181
22
IntelliJ IDE Java Python
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Just because I was curious I wrote this little utility to get started...


It works with some simple tests I ran... but there is probably room for improvement.
 
John Le
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That is pretty cool Steve. I'll have to dig into it a little deeper to get a better understanding of what you've done.

I've run a couple of quick tests against it and it has done exactly what I expected.

I'd swipe the whole thing from you except: 1) we're on 1.4, 2) I already checked in my inferior solution, and 3) I guess outright stealing your code is wrong.

Thanks for you help Steve!
 
Steve Luke
Bartender
Posts: 4181
22
IntelliJ IDE Java Python
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
For Java 1.4, you can take out all the generics stuff and replace the for loop with an iterator. It should be relatively easy I think.

As for 'stealing' my code - you (and anyone else) are free to take it if you like. I wouldn't put it here if it weren't free :-) So go ahead and take what you want.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!