views:

122

answers:

1

I have a pseudo-class in javascript that has a method to add and remove listeners to two buttons. This is the code:

function FirstObj(secondObj){
  this.loginButton = document.getElementById("login");
  this.logoutButton = document.getElementById("logout");
  this.secondObj = secondObj
}

FirstObj.prototype = {
  manageListeners : function(state){   //state is a boolean
    var self = this;
    if (state) {
      display += "none";
      this.loginButton.addEventListener("click", function(){
        self.seconfObj.makeSomething();
      }, false);
      this.logoutButton.removeEventListener("click", /*???*/ , false);
    }
    else {
      this.logoutButton.addEventListener("click", function(){
        self.logout();
      }, false);
      this.loginButton.removeEventListener("click", /*???*/ , false);
    }
  }, 
  logout : function(){
    //logout...
  }
}

the question is: how i could modify this code to manage event listener correctly?

+1  A: 

I don't think you can remove an event listener for an anonymous function. Use named functions instead:

var myEventListener = function() {
    console.log("Hello World!");
};

myElement.addEventListener("click", myEventListener, false);
myElement.removeEventListener("click", myEventListener, false);

Here is a slightly modified version of your class that should manage the event listeners properly:

function FirstObj(secondObj){

    var self = this;

    this.loginButton = document.getElementById("login");
    this.logoutButton = document.getElementById("logout");
    this.secondObj = secondObj;

    this.loginButtonClicked = function(){
        self.secondObj.makeSomething();
    };

    this.logoutButtonClicked = function(){
        self.logout();
    };

}

FirstObj.prototype = {

    manageListeners : function(state){

        if (state) {
            display += "none";
            this.loginButton.addEventListener("click", this.loginButtonClicked, false);
            this.logoutButton.removeEventListener("click", this.logoutButtonClicked, false);
        }
        else {
            this.logoutButton.addEventListener("click", this.logoutButtonClicked, false);
            this.loginButton.removeEventListener("click", this.loginButtonClicked, false);
        }

    },

    logout : function(){
        // Log out...
    }

};

If you rewrite your class to take advantage of closures, though, you can simplify it to this:

function FirstObj(secondObj){

    var self = this;

    var loginButton = document.getElementById("login");
    var logoutButton = document.getElementById("logout");

    var loginButtonClicked = function(){
        secondObj.makeSomething();
    };

    var logoutButtonClicked = function(){
        self.logout();
    };

    this.manageListeners = function(state){

        if (state) {
            display += "none";
            loginButton.addEventListener("click", loginButtonClicked, false);
            logoutButton.removeEventListener("click", logoutButtonClicked, false);
        }
        else {
            logoutButton.addEventListener("click", logoutButtonClicked, false);
            loginButton.removeEventListener("click", loginButtonClicked, false);
        }

    };

    this.logout = function(){
        // Log out...
    };

}

Here I've assumed that loginButton, logoutButton, and secondObj don't need to be accessed from outside the class. If they do, just make them properties of FirstObj and update the code that references them (using this in scope and self out of scope).

Steve Harrison
yes i know this, but if i give a name to them, then i have the problem of "this" object, and anonymous functions become unnecessary
Manuel Bitto
@Kucebe - That may be, but you still can't remove an anonymous listener, not unless you indiscriminately unbind all handlers for the event. Named, whatever form, is your best bet.
Nick Craver
@Kucebe: I believe you have the `this` problem whether the function is anonymous or not. I always simply declare a variable `_this` in the correct scope and use this to reference `this` whenever I'm in a function.
Steve Harrison
I can't give a name to those functions, the only way could be use global variables, but i don't want them because i'm doing it in OOP
Manuel Bitto
@Steve: the problem is the function that the event listener calls is in another scope (logout() function and secondOBJ.makeSomnething() ), so i need anonym function to preserv "this".If i use named function the problem of scope will transfer to it
Manuel Bitto
@Kucebe: I've updated my answer to show how you could modify the code to use named functions instead of anonymous functions. I'm a bit confused as to why `this` is a problem, since you never use it (you use `self` instead).
Steve Harrison
@Steve:yes, your code is right, but there is a second problem: every time the managerListener func is called, loginButtonClicked and logoutButtonClicked are overwritten; so i suppose that the removeEventListener doesn't remove exactly the same handler.
Manuel Bitto
In this way the listener is still there, loginButton fires event
Manuel Bitto
@Kucebe: OK, I've modified the code to move the named functions into the constructor function so they will only be declared once. To do this, I've made them methods of `FirstObj`. Notice, though, that they're declared where they can access `self` (and they will always be able to access `self` because a closure is created); if they were declared with all the other methods in `FirstObj.prototype = {...`, they wouldn't be able to access any local variables such as `self` and so we'd run into the `this` problem.
Steve Harrison
@Kucebe: If you don't want the named functions to be methods, you will have to rewrite the class so that it doesn't use `FirstObj.prototype = {...`.
Steve Harrison
@Kucebe: I've updated my answer to include a rewritten version of your class where the named functions aren't methods.
Steve Harrison
@Steve: Thanks Steve, your solution maybe is right but it doesn't work for my application, eventListener is always active, so i changed with DOM level 0 listener.
Manuel Bitto