Skip to content

Handle collision in the HashMap - Fixes #66 #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Handle collision in the HashMap - Fixes #66 #68

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 59 additions & 6 deletions src/data-structures/hash-table.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,21 @@
* HashTable.remove(10);
* HashTable.remove('key');
*
* console.log(HashTable.get(10)); // 'undefined'
* console.log(HashTable.get('key')); // 'undefined'
* console.log(HashTable.get(10)); // null
* console.log(HashTable.get('key')); // null
*
* @module data-structures/hash-table
*/
(function (exports) {
'use strict';

exports.Node = function (key, data) {
this.key = key;
this.data = data;
this.next = null;
this.prev = null;
};

exports.HashTable = function () {
this.elements = [];
};
Expand All @@ -41,25 +48,71 @@

for (i = 0; i < str.length; i += 1) {
character = str.charCodeAt(i);
/*jshint -W016 */
hashCode = ((hashCode << 5) - hashCode) + character;
hashCode = hashCode & hashCode;
/*jshint -W016 */
}

return hashCode;
};

exports.HashTable.prototype.put = function (key, value) {
exports.HashTable.prototype.put = function (key, data) {
var hashCode = this.hashCode(key);
this.elements[hashCode] = value;
var newNode = new Node(key, data);

if (this.elements[hashCode] === null) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this.elements[hashCode] might not be defined, I guess the check should be: this.elements[hashCode] === undefined

this.elements[hashCode] = newNode;
} else {
var first = this.elements[hashCode];

while (first.next !== null) {
first = first.next;
}

first.next = newNode;
newNode.prev = first;
}
};

exports.HashTable.prototype.get = function (key) {
var hashCode = this.hashCode(key);
return this.elements[hashCode];

if (this.elements[hashCode] === null) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

return null;
} else if (this.elements[hashCode].next === null) {
return this.elements[hashCode];
} else {
var first = this.elements[hashCode];

while (first.key !== key) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the code enters the body of this if statement and the key is not found (i.e. hasCode has returned the same value for key like the one returned for different key during insertion) and you loop over the linked list, you may have case in which first equals null (if you haven't found key but you've reached the end of the list). In this case the code will throw an error.
I believe you should also include the check:

while (first && first.key !== key) {
  first = first.next;
}

first = first.next;
}

return first;
}
};

exports.HashTable.prototype.remove = function (key) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove has the problems reported above.

var hashCode = this.hashCode(key);
this.elements.splice(hashCode, 1);

if (this.elements[hashCode] === null) {
return false;
} else if (this.elements[hashCode].next === null) {
this.elements.splice(hashCode, 1);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using splice(hashCode, 1) will remove the key from the array.
This means that if we have:
[{ key: "foo", value: "baz" }, { key: "bar", value: "foobar" }], hashCode has returned 0 for key foo and 1 for key bar. If you want to remove foo, according to the current implementation you'll use: splice(0, 1), this will change the elements array to:
[{ key: "bar", value: "foobar" }]. Next time you want to access the element with key bar and you invoke hashCode(bar), it will return 1, but since your array looks like [{ key: "bar", value: "foobar" }], you'll try to access the index 1, which value will be undefined.

You can use this.elements[hashCode] = undefined;.

return true;
} else {
var first = this.elements[hashCode];

while (first.key !== key) {
first = first.next;
}

if (first.next !== null) {
first.next.prev = first.prev;
}

first.prev.next = first.next;
}
};
})(typeof window === 'undefined' ? module.exports : window);