Javascript using for-in loop iterator to set a variable

Question

I want to dynamically create a button that will drop a key from an object. At this point however I am simply using an alert to test for the correct value that will be later passed to the function that will drop the key. I am running a for-in loop in and I am trying to pass the iterator to a function called in the loop. The problem is that the alert statement is using the iterator 'i' and as the loop ends all instances of this alert have been changed to the final value of 'i'. (I hope that makes sense!)

    locations = {};

function Location(nickname, address) {
    this.nickname = nickname;
    this.address = address;
}

Location.prototype.showLocations = function() {
    var x=document.getElementById("demo");
    output = "<table><tr><th>Location</th><th>Address</th><th>Delete</th></tr>";
    for (i in locations) (function(i)
    {
        output+=listThis(i);           

    }) (i);
    // for (i in locations) {
    //      output+=listThis(i);
    //     }
    output+="</table>"
    x.innerHTML=output;
}

function listThis(i){
    thisLoc = locations[i].nickname;
    var thisOutput="<tr><td>"+locations[thisLoc].nickname+"</td><td>"+locations[thisLoc].address+"</td><td><input type='button' value='X' onclick='alert(locations[thisLoc].nickname)' /></td></tr>";
    return thisOutput;
}

function enterLocation() {
    var address = document.getElementById('address').value;
    var nickname = document.getElementById('nickname').value;
    locations[nickname] = new Location(nickname, address);
    locations[nickname].showLocations();
}

The markup is:

<p id="demo">Table to go in here.</p>
<div id="panel">
    <input id="nickname" type="textbox" placeholder="Location Name" />
    <input id="address" type="textbox" placeholder="Sydney, NSW" />
    <input type="button" value="Enter" onclick="enterLocation()" />
</div>

Please note that I have tried to work with the info found at this post Javascript - how to work with the iterator in a for loop with callbacks but have not succeeded. You will see another for loop commented out that I was trying initially.

Don’t generate HTML in JavaScript; just manipulate the DOM instead. Also, could you add a jsFiddle or similar showing the problem, please? — Ry-
– Ry- ♦, Commented Oct 31, 2013 at 1:32
Please stop using global variables. Declare your variables locally. — jfriend00
– jfriend00, Commented Oct 31, 2013 at 1:39
@minitech How should I be dynamically adding rows to the table with DOM manipulation? document.createElement("tr") and then document.createElement("td")? Will put up a fiddle soon - I'll just sort the DOM manipulation first. — Khrys
– Khrys, Commented Oct 31, 2013 at 2:48

Ry- · Accepted Answer · 2013-10-31 01:46:41Z

2

The problem is in inline onclick handler.

You write onclick='alert(locations[thisLoc].nickname)' and here thisLoc is not a direct reference to your variable. It is some name which is evaluated at runtime.

On line thisLoc = locations[i].nickname; you define global variable thisLoc, which value is overwritten on each iteration. And later when onclick is processed this global variable with (always) latest value is accessed.

There're several solutions to that:

don't use HTML building as minitech said – use DOM manipulation

write value to some DOM attribute while building and read it in the onclick handler:

"<input type='button' value='X' data-location-name="' + thisLoc + '" onclick='alert(locations[this.getAttribute('data-location-name')].nickname)' />"

edited Oct 31, 2013 at 1:46

Ry-♦

226k56 gold badges496 silver badges504 bronze badges

answered Oct 31, 2013 at 1:37

kirilloid

14.4k6 gold badges42 silver badges54 bronze badges

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

1 Comment

Kevin Babcock Over a year ago

Why set an attribute with the value instead of just setting the value to be alerted directly?

Kevin Babcock · Accepted Answer · 2013-10-31 02:00:20Z

As others have mentioned, you need to properly scope your variables (i.e. don't use global variables).

var locations = {};

function Location(nickname, address) {
    this.nickname = nickname;
    this.address = address;
}

Location.prototype.showLocations = function() {
  var x = document.getElementById("demo");
  var output = "<table><tr><th>Location</th>" +
        "<th>Address</th><th>Delete</th></tr>";
  for (var i in locations) {
    output += listThis(i);
  }

  output += "</table>";
  x.innerHTML = output;
};

function listThis(i){
  var thisLoc = locations[i].nickname;
  var thisAddr = locations[i].address;
  var thisOutput= "<tr><td>" + thisLoc + "</td><td>" + thisAddr +
      "</td><td><input type='button' value='X' onclick='alert(\"" +
      thisLoc + "\")' /></td></tr>";
  return thisOutput;
}

function enterLocation() {
    var address = document.getElementById('address').value;
    var nickname = document.getElementById('nickname').value;
    locations[nickname] = new Location(nickname, address);
    locations[nickname].showLocations();
}

Here's a fiddle with the updated code.

It should be noted that this code is still adding the locations variable and all functions to the global scope, which is not a recommended practice. The only change required to solve your problems is to fix the scope of the variables defined and used inside your functions.

So how should I be storing the locations if not in the 'locations' global variable? Should I be using localStorage?

Collectives™ on Stack Overflow

Javascript using for-in loop iterator to set a variable

2 Answers 2

1 Comment

1 Comment

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

1 Comment

Linked

Related