CODEWITHSUNDEEP
javascriptloopsobjectaddeventlistener
Scratch Cat
Scratch Cat

Reputation: 59

For Loops With Objects JavaScript

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!

Upvotes: 1

Views: 62

Answers (4)

Scott Marcus
Scott Marcus

Reputation: 65873

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.

Upvotes: 1

Icewine
Icewine

Reputation: 1851

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>

Upvotes: 0

Isaiah Lee
Isaiah Lee

Reputation: 448

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.

Upvotes: 0

Hector Barbossa
Hector Barbossa

Reputation: 5528

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

Upvotes: 0

Related Questions