5

This is the most crazy thing I have seen in java (1.6):

Set<ActionPlan> actionPlans = assessment.getActionPlans();
//getActionPlans() returns a java.util.HashSet<ActionPlan>
ActionPlan actionPlan = actionPlans.iterator().next();
assertTrue(actionPlan1.equals(actionPlan));
assertEquals(actionPlan1.hashCode(), actionPlan.hashCode());
assertTrue(actionPlans.contains(actionPlan1));

The first two asserts pass but the last one fails.

I'm not giving you details on the ActionPlan and Assessment classes because it shouldn't matter. The contains method fails where the equals and hash don't.

I'm not saying that java is broken or anything, there is probably something funny going on in my code.

Note that I'm an experienced java programmer and I'm aware of the dos and don't for implementing equals and hashCode. So if something is missing in my code it's not something obvious.

Has anyone ever seen something that puzzling?

EDIT

I did some research in my code and I now think the problem is in hibernate. I have logged the hashCode of the ActionPlan object, after creation, and at different parts of the code until the failing assert is getting called. It does not change.

Also I have checked the class returned by assessment.getActionPlans() and it is:

org.hibernate.collection.internal.PersistentSet

I'm tempted to believe that this implementation of Set does not use equals or hashcode properly.

Does anyone have insight on that?

phoenix7360
  • 2,807
  • 6
  • 30
  • 41
  • Also note that the Assessment and ActionPLan objects are JPA (using hibernate implementation) entities retrieved using a persistence context of type TRANSACTION. – phoenix7360 Nov 28 '12 at 10:49
  • could you show equals and hashcode for `ActionPlan`? It should matter. – maasg Nov 28 '12 at 10:50
  • 2
    I think posting your code - in particular equals/hashCode is *key* to this – Brian Agnew Nov 28 '12 at 10:51
  • 1
    You've acknowledged that there's "probably something funny going on in [your] code" - and yet you think that code is irrelevant? Odd. – Jon Skeet Nov 28 '12 at 10:53
  • Well I don't want to blame java built in library without being sure so I'm willing to admit that there could be something wrong with my code. I think the answers lies in the first point of Peter's comment. I believe hibernate is giving me an implementation of a set that doesn't use equals or hashCode. – phoenix7360 Nov 28 '12 at 10:58
  • 1
    Depending on the class of the Set object, we may also need the compareTo or Comparator implementation, as well as the equals and hashCode declarations. For example, on the limited information so far, we could be dealing with a TreeSet and a class whose compareTo is inconsistent with equals. – Patricia Shanahan Nov 28 '12 at 10:59

3 Answers3

15

There is possible explainations

  • You have a sorted set which doesn't use equals or hashCode.
  • You have "overriden" equals(MyClass) instead of equals(Object)
  • The fields used by the hashCode are changed. This leave the Set in a state which is not usable.

The simplest way to test the last possibility is to try

assertTrue(new HashSet(actionPlans).contains(actionPlan1));

I suspect this will pass in your case. ;)


Date has a flaw that it is mutable and hashCode uses that mutable fields so you can corrupt any hash collection it is in by mutating it. A similar problem occurs when you alter a field which is used in compareTo.

Set<Date> dates = new HashSet<Date>();
SortedSet<Date> dates2 = new TreeSet<Date>();
Date d1 = new Date(1), d2 = new Date(2), d3 = new Date(3);
dates.add(d1);
dates.add(d2);
dates.add(d3);
dates2.add(d1);
dates2.add(d2);
dates2.add(d3);
d1.setTime(6);
d2.setTime(5);
d3.setTime(4);
System.out.print("The dates contains [");
for (Date date : dates) {
    System.out.print("date " + date.getTime() + " ");
}
System.out.println("]");
System.out.print("The sorted dates2 contains [");
for (Date date : dates2) {
    System.out.print("date " + date.getTime() + " ");
}
System.out.println("]");
for (int i = 1; i <= 6; i++)
    System.out.println("date " + i + " found is " + dates.contains(new Date(i))
            + " and " + dates2.contains(new Date(i)));

prints

The dates contains [date 6 date 5 date 4 ]
The sorted dates2 contains [date 6 date 5 date 4 ]
date 1 found is false and false
date 2 found is false and false
date 3 found is false and false
date 4 found is false and false
date 5 found is false and true
date 6 found is false and false

Note: the sorted collection is now in the wrong order.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • Indeed, your test passes :) So could it be that hibernate is returning some sort of homemade set that doesn't use equals or hashCode? – phoenix7360 Nov 28 '12 at 10:53
  • No, it means the last case is correct, i.e. the fields used in the hashCode were altered after they were added to the Set so it will act in an undefined way. – Peter Lawrey Nov 28 '12 at 10:57
  • Yeah I think that's probably it. Well spotted Peter! – phoenix7360 Nov 28 '12 at 11:00
  • I analysed my code (check the edit in my question) and the hashcode is never altered. I think the problem is in the implementation of set returned by hibernate (org.hibernate.collection.internal.PersistentSet) – phoenix7360 Nov 28 '12 at 12:05
5

This will happen if you overload equals but don't override equals(Object).

For example, you may have:

public boolean equals(ActionPlan plan) {
    ...
}

That will be called by:

assertTrue(actionPlan1.equals(actionPlan));

... but will not be called by contains. You need:

@Override public boolean equals(Object object) {
    ...
}

Of course, it's possible that that's not what's happening. There's no way we can tell for sure without seeing your code.

I'm not giving you details on the ActionPlan and Assessment classes because it shouldn't matter.

This answer contradicts that assumption... as does Peter's answer, which contains alternative failure modes. This is why giving a short but complete example is always important.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • As I mentioned, I know good practices and I did override (and not overload) equals. But thanks for the advice anyway. – phoenix7360 Nov 28 '12 at 10:54
  • 2
    @phoenix7360: Just because you *claim* to know good practices doesn't mean you actually *do* know them. That's why you should have provided your code (which is another good practice). – Jon Skeet Nov 28 '12 at 11:02
  • I understand your point. The reason I didn't provide my code is because those classes are quite big and so are the equals and hashCode methods. They also hold some business information that I'm not allowed to publish. Anyway, thanks again for the quick answers! – phoenix7360 Nov 28 '12 at 11:06
  • 1
    @phoenix7360: So that's why it would be worth trying to cut it down to a minimal example which demonstrated the problem, with no sensitive information. In the process of trying to do so, you may well have discovered the issue. – Jon Skeet Nov 28 '12 at 11:11
0

After I did equals and hashCode and made my keyField final, it still doesn't work. It took me another hour to find out that I needed this line in the "compareTo":

if (other != null && other.equals(this))
            return 0;
Cocu_1012
  • 109
  • 3