Bug and advice in Observer Pattern’s Codes
ElvisKang opened this issue · 0 comments
ElvisKang commented
Version:1.6.2
There are two issues in Observer Pattern's code:
1.(advice) the indexOf
method:
Original:
ObserverList.prototype.indexOf = function (obj, startIndex) {
var i = startIndex;
while (i < this.observerList.length) {
if (this.observerList[i] === obj) {
return i;
}
i++;
}
return -1;
};
I looked up the ECMASCRIPT 5.1 doc,and at 15.4.4.14, I found:
indexOf compares searchElement to the elements of the array, in ascending order, using the internal Strict Equality Comparison Algorithm (11.9.6),
So,I think that the code can be modified :
ObserverList.prototype.indexOf = function(obj,start){
var start = start || 0;
return this.list.indexOf(obj,start);
};
Test code:
var sub = new Subject();
var a = {x:1};
var b={x:2};
var c={x:3};
sub.addObserver(a);
sub.addObserver({x:2});
sub.addObserver(b);
sub.addObserver(c);
console.log(sub.observers);
console.log(sub.observers.indexOf(b));
Result:
{ observerList: [ { x: 1 }, { x: 2 }, { x: 2 }, { x: 3 } ] }
2
2.(bug) the Subject.prototype.removeObserver
method:
Code:
Subject.prototype.removeObserver = function (observer) {
this.observers.removeAt(this.observers.indexOf(observer,0));
};
If indexOf
method returns -1,then the last of element in observers
would be deleted incorrectly.
Test code:
var sub = new Subject();
var a = {x:1};
var b={x:2};
var c={x:3};
sub.addObserver(a);
sub.addObserver(b);
sub.addObserver(c);
console.log(sub.observers);
var d ={x:4};
sub.removeObserver(d);
console.log(sub.observers);
Result:
{ observerList: [ { x: 1 }, { x: 2 }, { x: 3 } ] }
{ observerList: [ { x: 1 }, { x: 2 } ] }