funerr
funerr

Reputation: 8176

JavaScript Named functions and Event handlers regarding CPU consumption

After re-factoring my code as @FlorianMargaine suggested(in the JavaScript Chat conversation) , I got something that looks like the following:

body.addEventListener( 'mousedown', action1);
function action1(){
    //Start selecting event
    body.addEventListener( 'mousemove', selectOver);
}
function selectOver(e){
    //after the user has selected and done a mouse up event show a box:
    body.addEventListener( 'mouseup', showBox );
}
function showBox(e){
    //Show box
    box.addEventListener( 'click', actionAlert('clicked on interface after selected text') );
}
function actionAlert(d){
    alert(d);
}

The main problem is that I think that it uses a lot of CPU on the way, how can I minimize that? I read a bit about the ability to remove the event handlers, is that the solution? and how can I integrate that solution efficiently to the code?

Upvotes: 0

Views: 174

Answers (2)

Pointy
Pointy

Reputation: 413936

(edit this is incorrect when using "addEventListener" but I'll leave it here as a historical curiosity:) You "action1" event handler re-binds the "mousemove" handler each time it's called. In turn, that handler binds a new handler for "mouseup". After a little while, there'll be hundreds and hundreds of redundant handlers.

So, the lesson is: don't bind event handlers within other event handlers (unless you really have a good reason). (edit — sorry; as I wrote above it's been pointed out that that's all incorrect. I'm used to binding handlers with jQuery, and that library does not behave the same way.)

Also: your "showBox" function is, as written, binding the result of calling the "actionAlert" method, which has no return value. I think what you want is:

box.addEventListener( 'click', function() {
  actionAlert('clicked on interface after selected text');
});

Upvotes: 3

Phrogz
Phrogz

Reputation: 303441

You should not be adding an event listener for mouseup on every mousemove, nor should you be re- registering your mousemove each time you mousedown Instead:

body.addEventListener( 'mousedown', action1, false);
function action1(){
    //Start selecting event
    body.addEventListener( 'mousemove', selectOver, false);
    body.addEventListener( 'mouseup', showBox, false );
    body.addEventListener( 'mouseup', function(){
      body.removeEventListener( 'mousemove', selectOver, false );
    });
}
function selectOver(e){
    // Not sure what this function is for.
}
function actionAlert(d){
    alert(d);
}

I've also added the explicit third parameter false to addEventListener as is required by some (all?) versions of Firefox.

Upvotes: 2

Related Questions