JS - dynamically created div containers contain the wrong data object

Question

I want to create some divs dynamically. Each div contains an object

function node(id, title, content, isPrivate, dateOfCreation) { // my node object
  this.id = id;
  this.title = title;
  this.content = content;
  this.isPrivate = isPrivate;
  this.dateOfCreation = dateOfCreation;
  this.lastEdited = dateOfCreation;
}

and this object is managed by my datastore class

    var store = new dataStore(); // instance of the store

    function dataStore() {
      this.nodes = []; // all the node objects

      this.getNodes = function() { // get all objects
        return this.nodes;
      }

  this.addNode = function(node) { // add a new object
    addElementToArray(this.nodes, node);
  }

  this.deleteNode = function(node) { // delete an object
    deleteElementFromArray(this.nodes, node)
  }

      this.getNodeById = function(nodeId){ // find an object by its id
        for(var i = 0; i < this.nodes.length; i++){
          if(nodeId == this.nodes[i].id)
                return this.nodes[i];
        }
        return null;
      }

      this.getNodeTitle = function(node){ // get the title of the node
        return node.title;
      }
    }

So I want to create some divs, one div for each object.

function initNodeView() { // build up all the divs

    for (var i = 0; i < 30; i++) { // -- TEST -- Fill the store
      store.addNode(new node(i, i, i, i % 2 == 0, i));
    }

  var nodes = store.getNodes(); // get all the nodes from the store

  for (var i = 0; i < nodes.length; i++) {
    var currentNode = nodes[i]; // the current object

    var nodeContainer = createDiv('#nodeList', "nodeContainer"); // the wrapperdiv

    createDivWithText(nodeContainer, "nodeContentDiv", store.getNodeTitle(currentNode)); // create a child div with the title of the object

    var barContainer = createDiv(nodeContainer, "nodeBtnBarDiv"); // create a childdiv - the container of all buttons

    createDivWithIcon(barContainer, "nodeIcon", currentNode.isPrivate ? "'foo.png'" : "'bar.png'"); // create an icon div

    var btnDefaultCss = "nodeBtn"; // default css class for buttons

    createBtn(barContainer, btnDefaultCss, "Edit", function() {
        // empty ..
    });

    createBtn(barContainer, btnDefaultCss, "Delete", function() {
      nodeContainer.remove(); // remove this container from the DOM
      store.deleteNode(currentNode); // delete this object from the store
    });
  }
}

To refactor my code I built up some helper functions.

function createDiv(parent, cssClass){ // create default div
    var divElement = $("<div></div>");
    divElement.addClass(cssClass);
    $(parent).append(divElement);
    return divElement;
}

function createDivWithText(parent, cssClass, text){ // create div with text
    var divElement = createDiv(parent, cssClass);
    divElement.html(text);
    return divElement;
}

function createDivWithIcon(parent, cssClass, iconSource){ // create icon div
    var divElement = createDiv(parent, cssClass);
    var icon = $("<img src='"+ iconSource +"'/>");
    divElement.append(icon);
    return divElement;
}

function createBtn(parent, cssClass, text, fn){ // create a new button
    var btn = $("<button></button>");
    btn.addClass(cssClass);
    btn.html(text);
    btn.click(function() {
        fn();
    });
    $(parent).append(btn);
}

My problem is, when calling initNodeView() the div containers are built up fine. But when clicking on a button (edit or delete) and want to log the stored object of this container it always got the last object stored in it.

So when creating 30 objects and 30 div containers they all got the 30th object stored in it.

Normally div 1 should have object 1, div 2 object 2, ....

Answer 1

The problem is that JavaScript uses the concept of function scope. This means that every variable declared inside a function is local to the function, but not as you might expect local to the current block of curly brackets. Thus there is only one variable currentNode that is assigned a new value in every iteration. But in your event handlers you refer to the same variable, whose final value is the last node.

for (var i = 0; i < nodes.length; i++) {
  var currentNode = nodes[i];
  // ...
}
// currentNode == nodes[nodes.length - 1]

There are many ways to solve this issue. You could wrap the body of your for-loop inside an anonymous function and invoke it immediately, to force JavaScript to create a new scope for each iteration.

for (var i = 0; i < nodes.length; i++) {
  (function(currentNode) {
     // ...
  })(nodes[i]);
}

Alternatively bind the value of currentNode to an argument of the event handlers.

for (var i = 0; i < nodes.length; i++) {
  var currentNode = nodes[i];
  // ...
  createBtn(barContainer, btnDefaultCss, "Edit", (function(currentNode) {
    // empty ..
  }).bind(null, currentNode));
}

For future projects you might consider the use the let keyword introduced by ECMAScript 6. Variables declared with let instead of var are scoped as you would expect.

for (let i = 0; i < nodes.length; i++) {
  let currentNode = nodes[i];
  // ...
}
// currentNode is not defined

According to caniuse.com this feature of JavaScript is implemented in pretty much all major browsers. But for sake of consistency you should avoid to mix let and var.

Answer 2

So I will just leave some working fiddles here, @just95 answered this very well too.

The first one is working with a seperate function to leave the scope:

for(var i = 0; i < 5; i++){
    var btn = $("<button></button>");
    btn.html(i);
    addClickEvent(btn, i);
    $('.container').append(btn)
}

function addClickEvent(btn, val){
    btn.click(function(){
      alert(val);
  });
};

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div class="container">

</div>

The second way is using the keyword "let" instead of "var"

for(let i = 0; i < 5; i++){
 var btn = $("<button></button>");
  btn.html(i);
  btn.click(function(){
   alert(i);
  });
 $('.container').append(btn)
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div class="container">

</div>

and the last one might be the best (in my opinion!) by using the jquery function each. When using native javascript you can use the forEach() function

var arr = ["one", "two", "three"];

$(arr).each(function(index, element){
 var btn = $("<button></button>");
  btn.html(element);
  btn.click(function(){
   alert(index + 1);
  });
 $('.container').append(btn)
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div class="container">

</div>