Simple HashMap remove function doesn't update the map entries-CodePudding

I'm trying to implement a simple hash map in which each entry of the map is represented as a Node single linked list, and I have an array of entries such as Node[] nodes:

static final class Node {
        int key;
        int value;
        Node next;
        int hash;

        Node(int key, int value, Node next, int hash) {
            this.key = key;
            this.value = value;
            this.next = next;
            this.hash = hash;
        }
}

Here is the remove method, which first tries to locate the index position of the key in the HashMap, and then scans linearly each node in this entry and removes the matched key in the map:

public void remove(int key) {
        int hash = computeHash(key);
        Node node = nodes[hash];
        if (node == null) {
            return;
        }

        Node prev = null;
        while (node.key != key && node.next != null) {
            prev = node;
            node = node.next;
        }
        if (node.key == key) {
            if (prev != null) {
                prev.next = node.next;
            } else if (prev == null) {
                node = node.next;
            } else {
                node = null;
            }
            size--;
        }
}

It doesn't make the matched node removed from the nodes outside and only the local variable referenced is affected. Since I have a reference to the node I want to remove it from the nodes (in case there is a matched key), having its address and trying to deference it to null, but it's invalid. I'm glad to hear some explanation why this code doesn't work.

I have a simple test like this:

MyHashMap map = new MyHashMap();
map.put(1, 1);
map.put(2, 1);
map.remove(1);
System.out.println(map.get(1)); // still returns 1

CodePudding user response：

You local variable node is a copy of nodes[hash]. Assigning to a copy doesn't change the original. You need to change your code so that it updates the original. And you don't need to check whether node.next equals null: it doesn't matter, if it's null you will assign null and that's precisely what you want.

if (node.key == key) {
    if (prev != null) {
        prev.next = node.next;
    } else {
        nodes[hash] = node.next;
    }
    size--;
}

CodePudding user response：

It's wrong to check node.next not equals null. You shouldn't care the next element, The next element from the removed one will never be referred. The source code from HashMap is:

if (node == p)
   tab[index] = node.next;
else
   p.next = node.next;
  modCount;
--size;

The 'p' is approach to your 'prev'.The HashMap assign the value to the first element in this hash earlier.