Skip to content
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.

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

Bug and advice in Observer Pattern’s Codes #158

Open
ElvisKang opened this issue Apr 21, 2015 · 0 comments
Open

Bug and advice in Observer Pattern’s Codes #158

ElvisKang opened this issue Apr 21, 2015 · 0 comments
Labels

Comments

@ElvisKang
Copy link

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 indexOfmethod 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 } ] }
@ElvisKang ElvisKang changed the title Bug and advice in Observer Pattern Bug and advice in Observer Pattern’s Codes Apr 21, 2015
@addyosmani addyosmani added the bug label Jul 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants