1

So I have this method that adds an object to a display. When the display is first opened all of the existing objects get added in to the HashMap 1 by 1. After that the user can add more, 1 at a time which puts a new object into the HashMap.

the Key is Object and the Value is a custom class that contains the Object and a couple of other variables about it.

This all works fine but when the display is closed and a new instance is opened it is supposed to reload all of the objects (including the ones that were created and saved by the user during use) but for some reason for anyone of the newly created ones, it finds an incorrect match when doing containsKey.

I don't really understand why it is finding matches when it should not. When I do containsValue it does not find any incorrect matches (it works as it should) but it doesn't help because when i use HashMap.put(K, V) it overrides the one that it would have returned the false positive for containsKey.

All the code does is iterate through each object, check if the HashMap contains the key already, if it does it returns the value for the key otherwise it creates a value based off the key passed in to the method and puts it into the hashmap.

I'm sorry I can't post code so if you can't help without it I understand but its for work and I'm not sure if it would be okay to post the code, even if its just a snippet.

Any help or guidance would be appreciated, I'm still searching around on google to see if I can find any information.

EDIT: I have found a solution. The hashcode function was returning a value which was essentially an index. The problem was that it got reset after the initial objects so the newly added ones would start from zero overwriting the existing ones. I modified the hashcode and its working.

Thank for the help everyone.

user1855475
  • 21
  • 4
  • 1
    If it's for work, then don't post the actual code, but you can write a small code snippet, demonstrating what you are actually doing. – Rohit Jain Nov 27 '12 at 15:29
  • I posted a modified snippet of what i think is the important part. – user1855475 Nov 27 '12 at 16:21
  • @user18554575.. What does your `node.getUserObject` returns? – Rohit Jain Nov 27 '12 at 16:30
  • Just the object that is associated with the node. Essentially it should be the same thing just as an Object class and without the parent, isFilterGroup, and isThecurrentEnum with it. – user1855475 Nov 27 '12 at 16:33
  • Regarding your edit: That is *not* a complete fix. Distinct values of `hashCode` will prevent a `HashMap` from seeing the two objects as equal, but `HashMap` *always* checks the `equals` method as well. So if changing the `hashCode` seemed to fix the problem, then that means your `equals` method was also broken, and still needs to be fixed! – ruakh Nov 28 '12 at 01:12

2 Answers2

5

Seems like you have overrided equals method but not the hashCode method in your class.

Note that, if you are using your custom class object as key in your HashMap, make sure that your class overrides both equals and hashCode method, else you would never be able to find the key in the HashMap again.

If you override equals but not hashCode, then your keys might be equal based on your equals method, but its hashCode will be generated from the hashCode method of Object class method, which generates different hashCode for your instances.

In general, if you should either override both equals and hashCode methods, or override none of them. Also, for calculating the hashCodes, you should use only those attributes, that you used for comparison in equals method.

Also, as from one of the comments, you should make sure that your Keys are immutable.

Rohit Jain
  • 209,639
  • 45
  • 409
  • 525
  • 1
    Also, keys should be immutable - if you change the state of an object used as a key in such a way that it influences the result of `equals` and `hashCode`, the map will get confused and you get unpredictable results. – Jesper Nov 27 '12 at 15:18
  • Re: "Seems like you have not overrided `equals` and `hashCode` method in your class": I think you have that backward. It seems like this class *has* overridden those methods; if it hadn't, then the `HashMap` would effectively be an `IdentityHashMap`, and `containsKey` could never return any false positives. – ruakh Nov 27 '12 at 15:19
  • @ruakh.. Yeah, actually I missed that. Edited my psot. – Rohit Jain Nov 27 '12 at 15:22
  • The custom class is the value. the top level 'Object' class is the key. – user1855475 Nov 27 '12 at 15:24
  • @user1855475. Please post some code also in your question, so that we can be clear what you have done. – Rohit Jain Nov 27 '12 at 15:26
  • @user1855475.. And why are you using `Object` as `key`? How are you putting objects in it? – Rohit Jain Nov 27 '12 at 15:27
  • That's just how it was. I didn't write this code, I've just been assigned to fix a bug which is caused by the false positives. – user1855475 Nov 27 '12 at 16:01
  • @user1855475.. Bro, we really can't help without seeing the code. It's not possible to guess where exactly you went wrong. I'm sorry. :( – Rohit Jain Nov 27 '12 at 16:10
  • The method is passed in an Object, then creates the custom object from that and a few other variables passed in. – user1855475 Nov 27 '12 at 16:12
  • ok i added a modified snippet. i hope thats enough. this is the part where the nodes are added in (this method is called in a loop for each key/value to be added to the hash map and again everytime the user adds a new one to the existing list) – user1855475 Nov 27 '12 at 16:18
  • Re: the updated "Seems like you have overrided `equals` method but not the `hashCode` method in your class": This is still wrong. Failing to override `hashCode` can result in false *negatives*, but never in false *positives*. – ruakh Nov 28 '12 at 01:14
1

I would suggest that you don't use a Custom Object as a key of your HashMap.

These objects have some identification? Some attribute that doesn't repeat from one object to another? Change the key of the HashMap to this attribute.

This way you can use the .containsKey() passing this attribute as parameter.

Renato Lochetti
  • 4,558
  • 3
  • 32
  • 49
  • They have to have a unique name when prompted to add a new one. I could try changing it to that but there's some infrastructure around the code that I will have to look at. – user1855475 Nov 27 '12 at 15:24