holyredbeard
holyredbeard

Reputation: 21218

Closures issue. Any tips?

I have an problem with the code below that's related to closures, that I need some help with.

As you can see I am creating several images in the for-loop that i'm assigning different id's (ie. the numbers from the array). So far, so good. When I click on the different images I want the function showId to be called with the images id's as argument, but the problem is that the number that's used as an argument to the function always becomes nr 8 (the last number in the array). How can I solve this?

Thanks in advance.

var imageArea = document.getElementById('imageArea');
var nrArray = [1,2,3,4,5,6,7,8];

for (var i = 0; i < nrArray.length; i++){
    var image = document.createElement('img');
    image.src = "images/theimage.png";
    image.id = nrArray[i];
    imgArea.appendChild(image);
    image.addEventListener ('click',function() {showId(image.id);},false);
}

Upvotes: 3

Views: 132

Answers (5)

J. Holmes
J. Holmes

Reputation: 18546

There are a bazillion answers to this exact same question here on stackoverflow, i'll hunt some down in a minute, but here are the key points:

  • Javascript has lexical scope (not dynamic scoping)
  • The smallest Javascript scope in javascript is a function
  • Block do not have thier own scope
  • Javascript uses variable hoisting, which means all variables in a scope are found before the scope executes and moved to the beginning of the scope. Which means that var image = ... isn't where you think it is. There is only one image variable, which (due to lexical scoping) is the variable you are accessing in your closure, which at the conclusion of the loop will obviously point at the last iterated item.
  • Functions are first-class objects, which means they are treated like variables and can be assigned and passed around as such
  • you can create a stable scope for a variable to live in by creating a self-executing anonymous function

So for example:

(function(localImage) {
    image.addEventListener ('click',function() {showId(localImage.id);},false);
})(image);

Also, as others have pointed out, event listener closures are executed in the context that they were bound. So conversely, without having worry about using a close to fix a scope, you could do:

image.addEventListener ('click',function() {showId(this.id);},false);

Edit

Some links to similar questions and some different perspectives on the answer:

I could go on but a bazillion is a big number...

Upvotes: 3

Pirokiko
Pirokiko

Reputation: 309

Javascript does not automatically remove variables from memory once the code goes out of scope. This means that even if you declare your variables inside a specific scope, in your case the for loop, outside of this the variable still exists.

The event listener attached to all your image is executing the line $showId(image.id); However because the variable isn't removed from memory the variable image still exists, namely the last one from your for loop. This is why you always get the last number from your array: 8.

When an eventlistener is called it has the variable this set to its own element, so changing the variable from image to this will fix the problem. The addEventListener call becomes:

image.addEventListener ('click',function() {showId(image.id);},false);

Upvotes: 0

David Brainer
David Brainer

Reputation: 6331

Why not just use this?

var imgArea = document.getElementById('imageArea');
var nrArray = [1,2,3,4,5,6,7,8];

for (var i = 0; i < nrArray.length; i++) {
    var image = document.createElement('img');
    image.src = "images/theimage.png";
    image.id = nrArray[i];
    imgArea.appendChild(image);
    image.addEventListener('click', function() {
        showId(this.id);
    }, false);
}

Also, your original question asked about creating closure, so even though I am sure that using this will provide what you need for now I am going to add a little demonstration of creating a new scope that will allow you to take advantage of closure to accomplish the same task:

var imgArea = document.getElementById("imageArea");
var nrArray = [1, 2, 3, 4, 5, 6, 7, 8];

for (var i = 0; i < nrArray.length; i++) {
    createClosure(i);
}

function createClosure(i) {
    var image = document.createElement('img');
    image.src = "images/theimage.png";
    image.id = nrArray[i];
    imgArea.appendChild(image);

    image.addEventListener('click', function () {
        showId(image.id);
    }, false);
}

jsFiddle demo: http://jsfiddle.net/HMYsW/1/

Upvotes: 1

Tomalak
Tomalak

Reputation: 338228

James Hill's answer points the easy way out of this situation, but of course you still have a closure issue. As soon as the data you want to use in the event listener is not available as a part of the environment (like, being a property of this), you'll have the same problem again.

To solve it, you create a new closure (i.e, a function):

function createListener(id) {
  return function() { showId(id); }
}

var imageArea = document.getElementById('imageArea');

for (var i = 1; i <= 8; i++){
    var image = document.createElement('img');
    image.src = "images/theimage.png";
    image.id = i;

    imgArea.appendChild(image);
    image.addEventListener ('click', createListener(image.id), false);
}

Here I have a function that creates and returns another function. The returned function will be the actual event listener. During its creation, the variable id is closed over and keeps its value. Of course you could do it in one step:

image.addEventListener ('click', function (id) {
 return function() { showId(id); };
}(image.id), false);

Upvotes: 0

James Hill
James Hill

Reputation: 61812

Simply access the ID by refencing the parent object using the this keyword:

//In this case, this refers to the object that owns the function, i.e., your img
image.addEventListener ('click',function() {showId(this.id);},false);

Upvotes: 3

Related Questions