1

How is the following possible:

void contains(LinkedHashSet data, Object arg) {
    System.out.println(data.getClass()); // java.util.LinkedHashSet
    System.out.println(arg.hashCode() == data.iterator().next().hashCode()); // true
    System.out.println(arg.equals(data.iterator().next())); // true
    System.out.println(new ArrayList(data).contains(arg)); // true
    System.out.println(new HashSet(data).contains(arg)); // true
    System.out.println(new LinkedHashSet(data).contains(arg)); // true (!)
    System.out.println(data.contains(arg)); // false
}

Am I doing something wrong?

Obviously, it doesn't always happen (if you create a trivial set of Objects, you won't reproduce it). But it does always happen in my case with more complicated class of arg.

EDIT: The main reason why I don't define arg here is that's it's fairly big class, with Eclipse-generated hashCode that spans 20 lines and equals twice as long. And I don't think it's relevant - as long as they're equal for the two objects.

Konrad Garus
  • 53,145
  • 43
  • 157
  • 230
  • 1
    If the arguments are `set` and `arg`, what is `data`? And what kind of Objects are you putting inside? If it's your own objects, did you override `equals()` and `hashCode()` properly? – stivlo Nov 04 '11 at 10:21
  • 1
    Also why do you check for `contains(file)` in the second to last line, but `contains(arg)` in the last? what is `file`? – pushy Nov 04 '11 at 10:59
  • stivlo, pushy - both are just typos on copy/paste, fixed. – Konrad Garus Nov 04 '11 at 11:05

3 Answers3

4

When you build your own objects, and plan to use them in a collection you should always override the following methods:

boolean equals(Object o);
int hashCode();

The default implementation of equals checks whether the objects point to the same object, while you'd probably want to redefine it to check the contents.

As much as is reasonably practical, the hashCode method defined by class Object does return distinct integers for distinct objects. To respect the rules an hashCode of an object equals to another one should be the same, thus you've also to redefine hashCode.

EDIT: I was expecting a faulty hashCode or equals implementation, but since your answer, you revealed that you're mutating the keys after they are added to an HashSet or HashMap.

When you add an Object to an hash collection, its hashCode is computed and used to map it to a physical location in the Collection.

If some fields used to compute the hashCode are changed, the hashCode itself will change, so the HashSet implementation will become confused. When it tries to get the Object it will look at another physical location, and won't find the Object. The Object will still be present if you enumerate the set though.

For this reason, always make HashMap or HashSet keys Immutable.

stivlo
  • 83,644
  • 31
  • 142
  • 199
  • well, I've read it, in fact I've spotted that you were data, set and arg, while you meant only data and arg. to be helped further you should show your arg class and how you implement equals and hashCode, but since your mean comment, don't expect help from me. – stivlo Nov 04 '11 at 10:50
  • That arg/data discrepancy was a good catch, but only a copy/paste oversight on my side. My harsh reaction was caused by the fact that from the description it is clear I am aware about hashCode and equals, and that they work properly in this case. – Konrad Garus Nov 04 '11 at 10:59
  • 1
    ok, then let's cool down. I think that without seeing the Object, it's not possible to answer this question. – stivlo Nov 04 '11 at 11:01
  • If the class overrides the equals method, and the hashCode contract demands that equal objects have equal hash codes. – Ozil Jun 12 '18 at 13:44
1

Got it. Once you know it, the answer is so obvious you can only blush in embarrassment.

static class MyObj {
    String s = "";

    @Override
    public int hashCode() {
        return s.hashCode();
    }

    @Override
    public boolean equals(Object obj) {
        return ((MyObj) obj).s.equals(s);
    }
}

public static void main(String[] args) {
    LinkedHashSet set = new LinkedHashSet();
    MyObj obj = new MyObj();
    set.add(obj);
    obj.s = "a-ha!";
    contains(set, obj);
}

That is enough to reliably reproduce it.

Explanation: Thou Shalt Never Mutate Fields Used For hashCode()!

Konrad Garus
  • 53,145
  • 43
  • 157
  • 230
0

There seems to be something missing from your question. I have made some guesses:

private void testContains() {
  LinkedHashSet set = new LinkedHashSet();
  String hello = "Hello!";
  set.add(hello);
  contains(set, hello);
}

void contains(LinkedHashSet data, Object arg) {
  System.out.println(data.getClass()); // java.util.LinkedHashSet
  System.out.println(arg.hashCode() == data.iterator().next().hashCode()); // true
  System.out.println(arg.equals(data.iterator().next())); // true
  System.out.println(new ArrayList(data).contains(arg)); // true
  System.out.println(new HashSet(data).contains(arg)); // true
  System.out.println(new LinkedHashSet(data).contains(arg)); // true (!)
  System.out.println(data.contains(arg)); // true (!!)
}

EDITED: To keep track of changing question!

I still get "true" for ALL but the first output. Please be more specific about the type of the "arg" parameter.

OldCurmudgeon
  • 64,482
  • 16
  • 119
  • 213