Code cleanup of Linked Lists (#5241)

* made private methods limited to module scope
* moved private methods to file bottom
* changed tests to run comperable array functions in parallel
* added comments
browser-messaging-saver
Cameron Fischer 2020-12-09 04:46:35 -05:00 zatwierdzone przez GitHub
rodzic 1e1aeefd93
commit cd5d9bd5b9
Nie znaleziono w bazie danych klucza dla tego podpisu
ID klucza GPG: 4AEE18F83AFDEB23
2 zmienionych plików z 99 dodań i 75 usunięć

Wyświetl plik

@ -24,40 +24,19 @@ LinkedList.prototype.clear = function() {
LinkedList.prototype.remove = function(value) {
if($tw.utils.isArray(value)) {
for(var t=0; t<value.length; t++) {
this._removeOne(value[t]);
_removeOne(this,value[t]);
}
} else {
this._removeOne(value);
_removeOne(this,value);
}
};
LinkedList.prototype._removeOne = function(value) {
var node = this.index[value];
if(node) {
node.prev.next = node.next;
node.next.prev = node.prev;
this.length -= 1;
// Point index to the next instance of the same value, maybe nothing.
this.index[value] = node.copy;
}
return node;
};
LinkedList.prototype._linkToEnd = function(node) {
// Sticks the given node onto the end of the list.
this.prev.next = node;
node.prev = this.prev;
this.prev = node;
node.next = this;
this.length += 1;
};
LinkedList.prototype.push = function(/* values */) {
for(var i = 0; i < arguments.length; i++) {
var value = arguments[i];
var node = {value: value};
var preexistingNode = this.index[value];
this._linkToEnd(node);
_linkToEnd(this,node);
if(preexistingNode) {
// We want to keep pointing to the first instance, but we want
// to have that instance (or chain of instances) point to the
@ -75,11 +54,11 @@ LinkedList.prototype.push = function(/* values */) {
LinkedList.prototype.pushTop = function(value) {
if($tw.utils.isArray(value)) {
for(var t=0; t<value.length; t++) {
this._removeOne(value[t]);
_removeOne(this,value[t]);
}
this.push.apply(this, value);
this.push.apply(this,value);
} else {
var node = this._removeOne(value);
var node = _removeOne(this,value);
if(!node) {
node = {value: value};
this.index[value] = node;
@ -95,7 +74,7 @@ LinkedList.prototype.pushTop = function(value) {
this.index[value] = node.copy;
node.copy = undefined;
}
this._linkToEnd(node);
_linkToEnd(this,node);
}
};
@ -113,6 +92,27 @@ LinkedList.prototype.toArray = function() {
return output;
};
function _removeOne(list,value) {
var node = list.index[value];
if(node) {
node.prev.next = node.next;
node.next.prev = node.prev;
list.length -= 1;
// Point index to the next instance of the same value, maybe nothing.
list.index[value] = node.copy;
}
return node;
};
function _linkToEnd(list,node) {
// Sticks the given node onto the end of the list.
list.prev.next = node;
node.prev = list.prev;
list.prev = node;
node.next = list;
list.length += 1;
};
exports.LinkedList = LinkedList;
})();

Wyświetl plik

@ -8,6 +8,10 @@ Tests the utils.LinkedList class.
LinkedList was built to behave exactly as $tw.utils.pushTop and
Array.prototype.push would behave with an array.
Many of these tests function by performing operations on a LinkedList while
performing the equivalent actions on an array with the old utility methods.
Then we confirm that the two come out functionally identical.
\*/
(function(){
@ -17,66 +21,88 @@ Array.prototype.push would behave with an array.
describe("LinkedList class tests", function() {
// pushTops a value or array of values into both the array and linked list.
function pushTop(array, linkedList, valueOrValues) {
$tw.utils.pushTop(array, valueOrValues);
linkedList.pushTop(valueOrValues);
};
// pushes values into both the array and the linked list.
function push(array, linkedList/*, other values */) {
var values = Array.prototype.slice(arguments, 2);
array.push.apply(array, values);
linkedList.push.apply(linkedList, values);
};
// operates a remove action on an array and a linked list in parallel.
function remove(array, linkedList, valueOrValues) {
$tw.utils.removeArrayEntries(array, valueOrValues);
linkedList.remove(valueOrValues);
};
// compares an array and a linked list to make sure they match up
function compare(array, linkedList) {
expect(linkedList.toArray()).toEqual(array);
expect(linkedList.length).toBe(array.length);
};
it("can pushTop", function() {
var array = [];
var list = new $tw.utils.LinkedList();
list.push('A', 'B', 'C');
push(array, list, 'A', 'B', 'C');
// singles
list.pushTop('X');
list.pushTop('B');
expect(list.toArray()).toEqual(['A', 'C', 'X', 'B']);
expect(list.length).toBe(4);
pushTop(array, list, 'X');
pushTop(array, list, 'B');
compare(array, list); // A C X B
//arrays
list.pushTop(['X', 'A', 'G', 'A']);
pushTop(array, list, ['X', 'A', 'G', 'A']);
// If the pushedTopped list has duplicates, they go in unempeded.
expect(list.toArray()).toEqual(['C', 'B', 'X', 'A', 'G', 'A']);
expect(list.length).toBe(6);
compare(array, list); // C B X A G A
});
it("can pushTop with tricky duplicates", function() {
var array = [];
var list = new $tw.utils.LinkedList();
list.push('A', 'B', 'A', 'C', 'A', 'end');
push(array, list, 'A', 'B', 'A', 'C', 'A', 'end');
// If the original list contains duplicates, only one instance is cut
list.pushTop('A');
expect(list.toArray()).toEqual(['B', 'A', 'C', 'A', 'end', 'A']);
expect(list.length).toBe(6);
pushTop(array, list, 'A');
compare(array, list); // B A C A end A
// And the Llist properly knows the next 'A' to cut if pushed again
list.pushTop(['X', 'A']);
expect(list.toArray()).toEqual(['B', 'C', 'A', 'end', 'A', 'X', 'A']);
expect(list.length).toBe(7);
pushTop(array, list, ['X', 'A']);
compare(array, list); // B C A end A X A
// One last time, to make sure we maintain the linked chain of copies
list.pushTop('A');
expect(list.toArray()).toEqual(['B', 'C', 'end', 'A', 'X', 'A', 'A']);
expect(list.length).toBe(7);
pushTop(array, list, 'A');
compare(array, list); // B C end A X A A
});
it("can handle particularly nasty pushTop pitfall", function() {
var array = [];
var list = new $tw.utils.LinkedList();
list.push('A', 'B', 'A', 'C');
list.pushTop('A'); // BACA
list.pushTop('X'); // BACAX
list.remove('A'); // BCAX
list.pushTop('A'); // BCXA
list.remove('A'); // BCX
push(array, list, 'A', 'B', 'A', 'C');
pushTop(array, list, 'A'); // BACA
pushTop(array, list, 'X'); // BACAX
remove(array, list, 'A'); // BCAX
pushTop(array, list, 'A'); // BCXA
remove(array, list, 'A'); // BCX
// But! The way I initially coded the copy chains, a mystery A could
// hang around.
expect(list.toArray()).toEqual(['B', 'C', 'X']);
expect(list.length).toBe(3);
compare(array, list); // B C X
});
it("can push", function() {
var array = [];
var list = new $tw.utils.LinkedList();
list.push('A', 'B', 'C');
push(array, list, 'A', 'B', 'C');
// singles
list.push('B');
expect(list.toArray()).toEqual(['A', 'B', 'C', 'B']);
expect(list.length).toBe(4);
push(array, list, 'B');
compare(array, list); // A B C B
// multiple args
list.push('A', 'B', 'C');
expect(list.toArray()).toEqual(['A', 'B', 'C', 'B', 'A', 'B', 'C']);
expect(list.length).toBe(7);
push(array, list, 'A', 'B', 'C');
compare(array, list); // A B C B A B C
});
it("can clear", function() {
@ -88,31 +114,29 @@ describe("LinkedList class tests", function() {
});
it("can remove", function() {
var array = [];
var list = new $tw.utils.LinkedList();
list.push('A', 'x', 'C', 'x', 'D', 'x', 'E', 'x');
push(array, list, 'A', 'x', 'C', 'x', 'D', 'x', 'E', 'x');
// single
list.remove('x');
expect(list.toArray()).toEqual(['A', 'C', 'x', 'D', 'x', 'E', 'x']);
expect(list.length).toBe(7);
remove(array, list, 'x');
compare(array, list); // A C x D x E x
// arrays
list.remove(['x', 'A', 'x']);
expect(list.toArray()).toEqual(['C', 'D', 'E', 'x']);
expect(list.length).toBe(4);
remove(array, list, ['x', 'A', 'x']);
compare(array, list); // C D E x
});
it('can ignore removal of nonexistent items', function() {
var array = [];
var list = new $tw.utils.LinkedList();
list.push('A', 'B', 'C', 'D');
push(array, list, 'A', 'B', 'C', 'D');
// single
list.remove('Z');
expect(list.toArray()).toEqual(['A', 'B', 'C', 'D']);
expect(list.length).toBe(4);
remove(array, list, 'Z');
compare(array, list); // A B C D
// array
list.remove(['Z', 'B', 'X']);
expect(list.toArray()).toEqual(['A', 'C', 'D']);
expect(list.length).toBe(3);
remove(array, list, ['Z', 'B', 'X']);
compare(array, list); // A C D
});
it('can iterate with each', function() {