Hash Map Object Key

Question

I'm using this class as my key to Hashmap with overriden hasCode() and equals()

public class Design {
private double[] factors = null;

public double[] getFactors() {
    return factors;
}

public void setFactors(double[] factors) {
    this.factors = factors;
}

public Design(double[] factors) {
    this.factors = factors;
}

@Override
public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + Arrays.hashCode(factors);
    return result;
}

@Override
public boolean equals(Object obj) {
    if (this == obj)
        return true;
    if (obj == null)
        return false;
    if (!(obj instanceof Design))
        return false;
    Design other = (Design) obj;
    if (!Arrays.equals(factors, other.factors))
        return false;
    return true;
}

I added values to map using a loop

 public static void main(String[] args) {

    Map<Design, Double> map = new HashMap<Design, Double>();

    double[] dbl = new double[2];

    for(int i=0; i<5; i++){
        for(int j=0; j<2; j++){
            System.out.println(j+i);
            dbl[j] = j+i;
        }
        Design des = new Design(dbl);
        map.put(des, Double.valueOf(i));
    }

    for(Design d: map.keySet()){
        System.out.println(d.getFactors()[0] + "\t" + d.getFactors()[1]);
    }

    for(double d: map.values()){
        System.out.println(d);
    }
}

The problem is in the key values. It displayed the last key added.

4.0 5.0
4.0 5.0
4.0 5.0
4.0 5.0
4.0 5.0

Where am I getting wrong?

thanks... workaround was to add dbl = new double[2]; before assigning new values to array... — JR Galia, Jan 09 '12 at 22:59

Sergey Kalinichenko · Answer 1 · 2012-01-09T23:00:28.327

You are not copying your double[] factors array in the setFactors and the constructor. That is why all instances of the key class end up sharing the same array that you modify in the loop.

You should change setFactors as follows:

public void setFactors(double[] factors) {
    this.factors = new double[factors];
    System.arrayCopy(factors, 0, this.factors, 0, factors.length);
}

public Design(double[] factors) {
    setFactors(factors);
}

Boann · Answer 2 · 2012-01-09T23:03:27.577

The problem is you're using the same array of doubles for all the instances you create of Design. When you initialize the next numbers in the main loop, you're modifying the first object again. Try this:

public double[] getFactors() {
    return factors.clone();
}

public void setFactors(double[] factors) {
    this.factors = factors.clone();
}

public Design(double[] factors) {
    setFactors(factors);
}

Or, if that would have a performance overhead that's not acceptable in your application, just be very careful about how you use the arrays passed to setFactors and the return value of getFactors.

score 2 · Answer 3 · answered Jan 09 '12 at 22:56

You've only made one array object. Java uses reference semantics for arrays, so every time you go through the loop, you change the values in the array—and the changes are reflected in all the Design objects.

In other words, you do have five different Design objects and it's printing them all, but they all reference the same array.

To fix this, make a new array for each iteration of the loop.

score 2 · Accepted Answer · answered Jan 09 '12 at 22:58

If you would move the declaration of your array into the for loop all would go as expected. The problem is now that all your Design instances have the same array.

for(int i=0; i<5; i++){
    double[] dbl = new double[2];
    for(int j=0; j<2; j++){
        System.out.println(j+i);
        dbl[j] = j+i;
    }
    Design des = new Design(dbl);
    map.put(des, Double.valueOf(i));
}

Furthermore, your equals method will yield to incorrect results when you have a subclass of Design. Instead of using instanceof, compare the classes. So change

if (!(obj instanceof Design))
        return false;

to

if (!(obj.getClass() == getClass() ) )
  return false;

This is however unrelated to your problem

Hash Map Object Key

4 Answers4