24

Have the following class:

public class Member {
private int x;
private long y;
private double d;

public Member(int x, long y, double d) {
    this.x = x;
    this.y = y;
    this.d = d;
}

@Override
public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + x;
    result = (int) (prime * result + y);
    result = (int) (prime * result + Double.doubleToLongBits(d));
    return result;
}

@Override
public boolean equals(Object obj) {
    if (this == obj) {
        return true;
    }
    if (obj instanceof Member) {
        Member other = (Member) obj;
        return other.x == x && other.y == y
                && Double.compare(d, other.d) == 0;
    }
    return false;
}

public static void main(String[] args) {
    Set<Member> test = new HashSet<Member>();
    Member b = new Member(1, 2, 3);
    test.add(b);
    System.out.println(b.hashCode());
    b.x = 0;
    System.out.println(b.hashCode());
    Member first = test.iterator().next();
    System.out.println(test.contains(first));
    System.out.println(b.equals(first));
           System.out.println(test.add(first));

}

}

It produces the following results:
30814 29853 false true true

Because the hashCode depends of the state of the object it can no longer by retrieved properly, so the check for containment fails. The HashSet in no longer working properly. A solution would be to make Member immutable, but is that the only solution? Should all classes added to HashSets be immutable? Is there any other way to handle the situation?

Regards.

catch23
  • 17,519
  • 42
  • 144
  • 217
robert
  • 243
  • 1
  • 2
  • 5
  • Why doesn't the compiler enforce a rule of not allowing mutable objects as keys in hash tables or objects in hash sets? This seems awful to me. – ncmathsadist Apr 22 '16 at 15:04

6 Answers6

34

Objects in hashsets should either be immutable, or you need to exercise discipline in not changing them after they've been used in a hashset (or hashmap).

In practice I've rarely found this to be a problem - I rarely find myself needing to use complex objects as keys or set elements, and when I do it's usually not a problem just not to mutate them. Of course if you've exposed the references to other code by this time, it can become harder.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • For value objects. Reference objects can be as mutable as you like. – Tom Hawtin - tackline Jan 17 '11 at 22:15
  • @Tom Hawtin: Wouldn't it also be unsafe to mutate reference objects as well. In particular, I speak of mutations that will change the object's hashcode or make it into a member of a different equivalence class. – Brian Jan 18 '11 at 01:43
  • 3
    @Brian: I suspect Tom is talking about types which don't override hashCode or equals. – Jon Skeet Jan 18 '11 at 07:09
  • @TomHawtin-tackline: If a hashset is used to encapsulate the states of objects, the objects in question should not be mutated during the lifetime of the hashset. If it is used to encapsulate the identities of objects, object identities are always immutable and thus won't mutate during the lifetime of the hashset. Unfortunately, Java makes no distinction between things which encapsulate object state and those which encapsulate object identity. – supercat Sep 19 '12 at 15:46
  • @supercat Can you clarify on what the distinction between "object identity" and the "object in question" is? I understand the former to mean "the reference" and the latter: the value of the object that a reference points at. – Muno Oct 23 '15 at 03:36
  • 1
    @Muno: I should have said "should not be mutated while residing in the hashset", but suppose one has a hash set of objects that have associated XY coordinates. If each hash table should only hold one object for any given location, it might sometimes be useful to have the location as a key, in which case the hash set could determine whether the it contained any object at a given XY coordinate pair. If the XY coordinates of an object in the hash table were to move, however, it may no longer be possible to retrieve the object from the hash table. – supercat Oct 23 '15 at 15:25
  • 1
    @Muno: Sometimes, however, it may be helpful to use a hash table to associate information with an object. For example, an object may wish to have a hash table listing all of the other objects that are connected to it in some fashion, and not want to have the nature of such connections change merely because some XY coordinates change. In that case, the table would be used to encapsulate the identify of the objects, rather than an aspect of their state (such as their location). – supercat Oct 23 '15 at 15:27
  • @JanezKuhar: Yes, that should have been "or" – Jon Skeet Aug 16 '19 at 22:17
9

Yes. While maintaining your class mutable, you can compute the hashCode and the equals methods based on immutable values of the class ( perhaps a generated id ) to adhere to the hashCode contract defined in Object class:

  • Whenever it is invoked on the same object more than once during an execution of a Java application, the hashCode method must consistently return the same integer, provided no information used in equals comparisons on the object is modified. This integer need not remain consistent from one execution of an application to another execution of the same application.

  • If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.

  • It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results. However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables.

Depending on your situation this may be easier or not.

class Member { 
    private static long id = 0;

    private long id = Member.id++;
    // other members here... 

     
    public int hashCode() { return this.id; }
    public boolean equals( Object o ) { 
        if( this == o ) { return true; }
        if( o instanceOf Member ) { return this.id == ((Member)o).id; }
        return false;
     }
     ...
 }

If you need a thread safe attribute, you may consider use: AtomicLong instead, but again, it depends on how are you going to use your object.

Community
  • 1
  • 1
OscarRyz
  • 196,001
  • 113
  • 385
  • 569
  • The problem with this approach is that if I insert two different instances in a HashSet (e.g. new Member(1,2, 3)) they will be inserted twice. Also the equals method will not work as expected. But thanks for pointing out the hashCode contract: "Whenever it is invoked on the same object more than once during an execution of a Java application, the hashCode method must consistently return the same integer, provided no information used in equals comparisons on the object is modified." I didn't read it carefully enough. – robert Jan 17 '11 at 21:54
  • Well that was just a sample, you could create it like this: `public Member( int a, int b, int ) { this.id = a * 31 + b * 31 + 31 *; }` If you create another `new Member(1,2,3)` it will compute the same hash, **but** if you modify one of them, you'll have to synchronize. You could use a registry/factory method and create objects like: `Member a = Member.newInstance(1,2,3); Member b = Member.newInstance(1,2,3);` and create the instance only once. – OscarRyz Jan 17 '11 at 23:10
  • The problem with this approach is that it's basically no different than using the == operator for equality. Every instance has a unique long id and comparing that vs comparing object instance references, basically the same thing. It doesn't test equality in contents. If one needs equality of contents for their case, this solution will not suffice – Achilles929 Mar 20 '18 at 12:07
4

As already mentioned, one can accept the following three solutions:

  1. Use immutable objects; even when your class is mutable, you may use immutable identities on your hashcode implementation and equals checking, eg an ID-like value.
  2. Similarly to the above, implement add/remove to get a clone of the inserted object, not the actual reference. HashSet does not offer a get function (eg to allow you alter the object later on); thus, you are safe there won't exist duplicates.
  3. Exercise discipline in not changing them after they've been used, as @Jon Skeet suggests

But, if for some reason you really need to modify objects after being inserted to a HashSet, you need to find a way of "informing" your Collection with the new changes. To achieve this functionality:

  1. You can use the Observer design pattern, and extend HashSet to implement the Observer interface. Your Member objects must be Observable and update the HashSet on any setter or other method that affects hashcode and/or equals.

Note 1: Extending 3, using 4: we may accept alterations, but those that do not create an already existing object (eg I updated a user's ID, by assigning a new ID, not setting it to an existing one). Otherwise, you have to consider the scenario where an object is transformed in such a way that is now equal to another object already existing in the Set. If you accept this limitation, 4th suggestion will work fine, else you must be proactive and define a policy for such cases.

Note 2: You have to provide both previous and current states of the altered object on your update implementation, because you have to initially remove the older element (eg use getClone() before setting new values), then add the object with the new state. The following snippet is just an example implementation, it needs changes based on your policy of adding a duplicate.

@Override
public void update(Observable newItem, Object oldItem) {
    remove(oldItem);
    if (add(newItem))
        newItem.addObserver(this);
}

I've used similar techniques on projects, where I require multiple indices on a class, so I can look up with O(1) for Sets of objects that share a common identity; imagine it as a MultiKeymap of HashSets (this is really useful, as you can then intersect/union indices and work similarly to SQL-like searching). In such cases I annotate methods (usually setters) that must fireChange-update each of the indices when a significant change occurs, so indices are always updated with the latest states.

Community
  • 1
  • 1
Kostas Kryptos
  • 4,081
  • 2
  • 23
  • 24
3

Jon Skeet has listed all alternatives. As for why the keys in a Map or Set must not change:

The contract of a Set implies that at any time, there are no two objects o1 and o2 such that

o1 != o2 && set.contains(o1) && set.contains(o2) && o1.equals(o2)

Why that is required is especially clear for a Map. From the contract of Map.get():

More formally, if this map contains a mapping from a key k to a value v such that (key==null ? k==null : key.equals(k)), then this method returns v, otherwise it returns null. (There can be at most one such mapping.)

Now, if you modify a key inserted into a map, you might make it equal to some other key already inserted. Moreover, the map can not know that you have done so. So what should the map do if you then do map.get(key), where key is equal to several keys in the map? There is no intuitive way to define what that would mean - chiefly because our intuition for these datatypes is the mathematical ideal of sets and mappings, which don't have to deal with changing keys, since their keys are mathematical objects and hence immutable.

meriton
  • 68,356
  • 14
  • 108
  • 175
0

Theoretically (and more often than not practically too) your class either:

  1. has a natural immutable identity that can be inferred from a subset of its fields, in which case you can use those fields to generate the hashCode from.
  2. has no natural identity, in which case using a Set to store them is unnecessary, you could just as well use a List.
biziclop
  • 48,926
  • 12
  • 77
  • 104
-1

Never change 'hashable field" after putting in hash based container.

As if you (Member) registered your phone number (Member.x) in yellow page(hash based container), but you changed your number, then no one can find you in the yellow page any more.

卢声远 Shengyuan Lu
  • 31,208
  • 22
  • 85
  • 130