1

I'm having a problem with looping at the moment with JavaScript. I have an object:

var object = {
    objectInObject: {
        name: "Banana"
    },
    objectInObject2: {
        name: "Apple"
    },
    objectInObject3: {
        name: "Carrot"
    }
}

And I am looping through the object's objects:

for(var key in object){
    var li = document.createElement('li');
    li.textContent = object[key].name;
    ul.appendChild(li);
    li.addEventListener('click', function(){
        console.log(object[key]);
    })
}

The problem I'm having is when I add an Event Listener and click on the list item for example "Banana", when I console.log it it still displays "Carrot". So no matter which list item I click, it just shows the latest one. I am not sure why this is happening. Any help would be very much appreciated!

  • http://stackoverflow.com/questions/111102/how-do-javascript-closures-work – Eeks33 May 10 '17 at 02:26
  • Possible duplicate of [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – 4castle May 10 '17 at 02:38

4 Answers4

1

This is happening because there is a closure around "key". The event handler that is being assigned to the three elements you are generating are all sharing the "key" variable from the parent function. The last value "key" gets is Carrot and so that is the value all the handlers share.

Change "var key" to "let key" to create block scope for "key" and avoid the closure. This change allows "key" to be entirely new upon each loop iteration so each event handler doesn't share "key" with any other.

You can read more about closures at: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures

Also, FYI, it's not a great idea to name a variable "object" as this can conflict with the "object" type.

Lastly, it's never a good idea to modify the DOM in a loop. Instead build up a documentFragment and then append that just once when the loop is finished.

Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
  • 1
    Hey it worked!!! But I'm not entirely sure why. I thought 'var' and 'let' would be the same in this case because their both in a 'for' scope? Maybe explain a little more because I don't really understand. Thank you so much for the tips. – Scratch Cat May 10 '17 at 02:38
  • Actually no. var doesn't give you for scope. var gives you function scope here and that's what causes the problem. let doesn't actually give you for scope either. It gives you a new scope for each iteration of for. – Scott Marcus May 10 '17 at 02:42
0

Change for(var key in object) to for(let key in object).

Hector Barbossa
  • 5,506
  • 13
  • 48
  • 70
0

Two things to note here.

First, it's good practice to use the Object's hasOwnProperty() method to ensure that you're accessing the key on that object, and not anything else higher up on the prototype chain. Like this:

for(var key in object){
    if (object.hasOwnProperty(key) {
        // do whatever you need with object[key] here safely
    }       
}

Second, the reason why you're getting Carrot for all of them is that the value of key is "not being saved" as you iterate through the object. What's really happening is in your even listener callback, a reference to the key is being saved. By the time you go through all the objects, the last key reached becomes the same one all the event listeners are pointing to.

What you need can be done by introducing a closure, or more specifically, by creating a new execution environment. The simplest method would probably be an Immediately Invoked Function Expression (IIFE) here:

li.addEventListener('click', function(){
    (function(key) {
        console.log(key);
    })(object[key]);
})

What's happening here is that by creating a new function scope, the key is no longer being referenced by all the event listener callbacks. They all create and immediately execute a new function, each with their own scope, and with their own "key" values that you simply pass in.

Isaiah Lee
  • 448
  • 2
  • 10
0

This is how I would do it.

var object = {
    objectInObject: {
        name: "Banana"
    },
    objectInObject2: {
        name: "Apple"
    },
    objectInObject3: {
        name: "Carrot"
    }
}

var ul = document.getElementById("container");
for(var key in object){

    var li = document.createElement('li');
    
    li.textContent = object[key].name;
    li.setAttribute("data-id", object[key].name);
    
    li.addEventListener('click', function(){
        console.log(this.getAttribute("data-id"));
    });
    
    ul.appendChild(li);
    
}
li:hover {
  cursor: pointer;
}
<ul id='container'></ul>
Icewine
  • 1,851
  • 1
  • 12
  • 22