4

I have a Hashmap in which I wrote a class that deals with adding and retrieving values.

class ReputationMatrix
{
    private HashMap < Integer, int[] > repMatrix;

    public ReputationMatrix()
    {
        repMatrix = new HashMap < Integer, int[] > ();
    }

    public void addrating(int nodeId, boolean rating)
    {
        int[] alphaBeta;

        if (repMatrix.containsKey(nodeId))
        {
            alphaBeta = repMatrix.get(nodeId);

            if (rating == true)
            {
                alphaBeta[0] = alphaBeta[0] + 1;
            }
            else
            {
                alphaBeta[1] = alphaBeta[1] + 1;
            }

            repMatrix.put(nodeId, alphaBeta);
        }
        else
        {
            alphaBeta = new int[2];

            if (rating == true)
            {
                alphaBeta[0] = 2;
                alphaBeta[1] = 1;
            }
            else
            {
                alphaBeta[0] = 1;
                alphaBeta[1] = 2;
            }

            repMatrix.put(nodeId, alphaBeta);

        }
    }

    public int[] getNodeIds()
    {
        int[] nodeIds = new int[repMatrix.size()];
        int index = 0;

        for (int key: repMatrix.keySet())
        {
            nodeIds[index] = key;
            index++;
        }

        return nodeIds;
    }

    public int getAlpha(int nodeId)
    {
        return repMatrix.get(nodeId)[0];
    }

    public int getBeta(int nodeId)
    {
        return repMatrix.get(nodeId)[1];
    }

    public ReputationMatrix clone()
    {
        ReputationMatrix matrixClone = new ReputationMatrix();
        matrixClone.repMatrix.putAll(this.repMatrix);
        return matrixClone;
    }
}

I implemented a clone method to simply return a separate copy of ReputationMatrix totally independent from the original.

I tested the code like this:

public class Main
{
    /**
     * @param args
     */
    public static void main(String[] args)
    {
        ReputationMatrix matrix1 = new ReputationMatrix();
        matrix1.addrating(18, true);

        ReputationMatrix matrix2 = matrix1.clone();

        System.out.println(matrix1.getAlpha(18));
        System.out.println(matrix2.getAlpha(18));

        matrix1.addrating(18, true);

        System.out.println(matrix1.getAlpha(18));
        System.out.println(matrix2.getAlpha(18));
    }
}

the output was:

2
2
3
3

Which means every change I apply to matrix1 is reflecting onto matrix2. I'm almost sure putAll does create copies. What am I doing wrong?

user1378063
  • 263
  • 1
  • 6
  • 9
  • 1
    `.putAll()` never does "deep copies"; it only does shallow copies. Where did you read that it did deep copies? – fge Apr 08 '15 at 14:03
  • put and putall just stores the references to the objects. – Dan Apr 08 '15 at 14:04
  • Maybe I was wrong then. So the only way I can clone the Hashmap is if I loop through all keys and clone the arrays? – user1378063 Apr 08 '15 at 14:06
  • 1
    In Java you never get copies automatically, it must be explicit. There is a fundamental reason for it: copying an object graph is an ill-defined concept and cannot be solved for the general case. – Marko Topolnik Apr 08 '15 at 14:10

5 Answers5

4

From the documentation:

putAll

Copies all of the mappings from the specified map to this map (optional operation). The effect of this call is equivalent to that of calling put(k, v) on this map once for each mapping from key k to value v in the specified map.

So it does not make copies of the objects, it only adds the mappings from the original map to the new map.

To do what you want, you would need to copy each value explicitly:

Map<Integer, int[]> originalMatrix = new HashMap<>();
int[] original = {1, 2, 3};
originalMatrix.put(1, original);
Map<Integer, int[]> newMatrix = new HashMap<>();
    
for (Map.Entry<Integer, int[]> entry : originalMatrix.entrySet()) {
    newMatrix.put(entry.getKey(), entry.getValue().clone());
}

Arrays.fill(original, 0);

System.out.println(Arrays.toString(original));
System.out.println(Arrays.toString(newMatrix.get(1)));

Output:

[0, 0, 0]
[1, 2, 3]
Community
  • 1
  • 1
Anderson Vieira
  • 8,919
  • 2
  • 37
  • 48
  • I have folllowing data set `HashMap >>` and when I use putAll I looks like that is copied object not a value. Because changes in the first influence second one – murt Feb 08 '16 at 09:00
1

putAll doesn't create copies of the keys and the values. It calls put for each of the key/value pairs passed to it, and put doesn't create a copy.

Eran
  • 387,369
  • 54
  • 702
  • 768
1

No, putAll() does not clone the elements to the map. It just copies the reference to them so you have two reference variables pointing to the same object into the heap. This is called shallow copy. If you want to clone all the elements (deep copy) you have to do something like this:

Map<K,V> original = new HashMap<K,V>();
Map<K,V> clone = new HashMap<K,V>();
for(Map.Entry<K,V> entry : original.entrySet) {
  clone.put(entry.getKey(), entry.getValue().clone());
}
Kiril Aleksandrov
  • 2,601
  • 20
  • 27
0

As you can see, there is no copy.

 /**
 * Copies all of the mappings from the specified map to this map.
 * These mappings will replace any mappings that this map had for
 * any of the keys currently in the specified map.
 *
 * @param m mappings to be stored in this map
 * @throws NullPointerException if the specified map is null
 */
public void putAll(Map<? extends K, ? extends V> m) {
    int numKeysToBeAdded = m.size();
    if (numKeysToBeAdded == 0)
        return;

    /*
     * Expand the map if the map if the number of mappings to be added
     * is greater than or equal to threshold.  This is conservative; the
     * obvious condition is (m.size() + size) >= threshold, but this
     * condition could result in a map with twice the appropriate capacity,
     * if the keys to be added overlap with the keys already in this map.
     * By using the conservative calculation, we subject ourself
     * to at most one extra resize.
     */
    if (numKeysToBeAdded > threshold) {
        int targetCapacity = (int)(numKeysToBeAdded / loadFactor + 1);
        if (targetCapacity > MAXIMUM_CAPACITY)
            targetCapacity = MAXIMUM_CAPACITY;
        int newCapacity = table.length;
        while (newCapacity < targetCapacity)
            newCapacity <<= 1;
        if (newCapacity > table.length)
            resize(newCapacity);
    }

    for (Iterator<? extends Map.Entry<? extends K, ? extends V>> i = m.entrySet().iterator(); i.hasNext(); ) {
        Map.Entry<? extends K, ? extends V> e = i.next();
        put(e.getKey(), e.getValue());
    }
} 

This is original src from openjdk.

 for (Iterator<? extends Map.Entry<? extends K, ? extends V>> i = m.entrySet().iterator(); i.hasNext(); ) {
        Map.Entry<? extends K, ? extends V> e = i.next();
        put(e.getKey(), e.getValue());
    }

We just put each Key-Value to our map.

RSCh
  • 171
  • 6
0

There is no deep copy in Java. If you want it you have to code it by calling clone() recursively.

Note also that arrays are objects hence int[] in

private HashMap < Integer, int[] > repMatrix;

is a reference on a int array. When you get this array in addRating(), the hashmap still hold a reference on the array, and there is no need to put() after modification

    if (repMatrix.containsKey(nodeId)) {
        alphaBeta = repMatrix.get(nodeId);

        if (rating == true) {
            alphaBeta[0] = alphaBeta[0] + 1;
        }
        else {
            alphaBeta[1] = alphaBeta[1] + 1;
        }

        //repMatrix.put(nodeId, alphaBeta); <= not needed
    }
T.Gounelle
  • 5,953
  • 1
  • 22
  • 32