How to properly add addeventlistener?

Question

I try add to array with image event listener which catch load event and it's works. But sometimes i parameter passed to the decrement function is the same for all requests

var imgNumb = vectors.length;
function decrement(i, type){
    imgNumb--;
    console.log('Processed '+ type+' nr: ' + i + '. Left: '+ imgNumb);
}
for(var i in vectors)
{
    if(jQuery('#canvas-'+i).length != 0){
    var tempCont = document.getElementById('canvas-'+i);
    tempImage[i] = new Image();
    alert(i);
    tempImage[i].addEventListener('load', function(){
        decrement(i, 'canvas');
    }, false);
    type[i] = 'canvas';
       tempImage[i].src = tempCont.toDataURL();
    }
}

for example i get:

Processed canvas nr: 1. Left: 2
Processed canvas nr: 2. Left: 1
Processed canvas nr: 2. Left: 0

but alert (if it isn't inside handler) always return correct key number.

This is a classic closure problem

wong2
– wong2

2011-05-12 14:06:32 +00:00
Commented May 12, 2011 at 14:06 — wong2
– wong2, Commented May 12, 2011 at 14:06

Felix Kling · Accepted Answer · 2011-05-12 14:01:19Z

2

You are creating a function in a loop which is dependent on the loop variable. All functions reference the same i. You have to introduce a new scope (by calling a function) to capture the current value of the variable:

(function(index) {
    tempImage[index].addEventListener('load', function(){
        decrement(index, 'canvas');
    }, false);
}(i));

Here we use an immediate or self-invoking function.

Don't use for...in to iterate over arrays. Use a normal for loop.

answered May 12, 2011 at 14:01

Felix Kling

820k181 gold badges1.1k silver badges1.2k bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Matt Ball · Accepted Answer · 2011-05-12 14:11:40Z

1

@Felix's answer is correct, but I thought I might show an alternative solution that might be more readable:

var imgNumb = vectors.length,
    tempImage = [];

function decrement(i, type) {
    imgNumb--;
    console.log('Processed ' + type + ' nr: ' + i + '. Left: ' + imgNumb);
}

$.each(vectors, function(i, element) {
    var $canvas = $('#canvas-' + i);
    if ($canvas.length) {
        tempImage[i] = $('<img/>', {
            src: $canvas.get().toDataURL(),
            load: function() {
                decrement(i, 'canvas');
            }
        });
        type[i] = 'canvas';
    }
});

It's not clear in the question if vectors is a jQuery object or a plain JS array, so I've taken the safe assumption and used $.each(). If vectors is a jQuery object, you can use .each() instead.

Other fixes:

Don't use for... in to iterate over arrays.
Just check the truthiness of jQuery('#canvas-' + i).length
You're using jQuery - why use document.getElementById()?
Again, you're using jQuery - why use addEventListener()?

answered May 12, 2011 at 14:11

Matt Ball

361k102 gold badges655 silver badges725 bronze badges

3 Comments

Felix Kling Over a year ago

+1 I didn't see that the OP is using jQuery... then it can definitely be improved a lot :)

sebastian Over a year ago

Unfortunately you code doesn't work. Firstly what is that ->$canvas.get().toDataUrl()? 2nd, this

$('<img/>', {             src: $canvas.get().toDataURL(),             load: function() {                 decrement(i, 'canvas');             }         });

create a jQuery object, and canvas drawImage method throw the exception.

Matt Ball Over a year ago

Calling .get() on a jQuery object returns the underlying DOM element. To further solve your problem, it would help if you could show a more complete example of your code. I don't see anywhere that you're calling drawImage() on the canvas.

Collectives™ on Stack Overflow

How to properly add addeventlistener?

2 Answers 2

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related