CODEWITHSUNDEEP
javascriptfor-loop
janos
janos

Reputation: 124646

Cannot convert traditional for loop to for-each loop

For some inexplicable reason, I cannot convert this loop into a more natural for (var player in fizzPlayers) { ... } loop:

  for (var i = 0; i < fizzPlayers.length; i++) {
    var player = fizzPlayers[i];
    var val = parseInt(player.value);
    if (val != 0) {
      active.push(player);
      actfact.push(parseInt(player.value));
      actname.push(capitaliseFirstLetter(player.id));
    }
  }

I want to write this the more natural for-each way, like this:

  for (var player in fizzPlayers) {
    var val = parseInt(player.value);
    if (val != 0) {
      active.push(player);
      actfact.push(parseInt(player.value));
      actname.push(capitaliseFirstLetter(player.id));
    }
  }

But it doesn't work, as you can see in the runnable code snippet at the bottom.

For debugging, I inserted this code right before the loop:

  // prints undefined... but why?
  for (var player in fizzPlayers) {
    console.log(player.value);
  }

It prints undefined for the 5 players. Why? There is a similar loop earlier in the program where the for (var player in fizzPlayers) { ... } loop is working just fine.

Why is this happening? What am I missing?

var fizzLoaded = false;
var fizzDiv, fizzFrom, fizzTo, fizzPlayers;

function fizzLoad() {
  if (fizzLoaded) {
    return;
  }
  fizzLoaded = true;
  var fizzForm = document.getElementById('fizzbuzz');
  fizzFrom = document.getElementById('rangeFrom');
  fizzTo = document.getElementById('rangeTo');
  fizzPlayers = [
    document.getElementById('frodo'),
    document.getElementById('sam'),
    document.getElementById('merry'),
    document.getElementById('pippin'),
    document.getElementById('bilbo')
  ];
  fizzDiv = document.getElementById('fizzOut');
}

function restrictRange() {
  var rFrom = parseInt(fizzFrom.value);
  var rTo = parseInt(fizzTo.value);
  fizzTo.min = rFrom;
  fizzFrom.max = rTo;
}

function validateValues() {
  var rFrom = parseInt(fizzFrom.value);
  var rTo = parseInt(fizzTo.value);
  if (rTo < rFrom) {
    alert("Illegal range from " + rFrom + " to " + rTo);
    return false;
  }
  for (var player in fizzPlayers) {
    var val = parseInt(player.value);
    if (val < 0 || val > 100) {
      alert("Illegal value " + val + " for player " + player.id);
      return false;
    }
  }
  return true;
}

function capitaliseFirstLetter(string) {
      return string.charAt(0).toUpperCase() + string.slice(1);
}

function fizzing() {
  fizzLoad();
  restrictRange();
  if (!validateValues()) {
    fizzDiv.innerHTML = "Illegal inputs";
    return;
  }

  var table = "";
  var rFrom = parseInt(fizzFrom.value);
  var rTo = parseInt(fizzTo.value);
  var active = [];
  var actfact = [];
  var actname = [];

  // prints undefined... but why?
  for (var player in fizzPlayers) {
    console.log(player.value);
  }

  for (var player in fizzPlayers) {
    var val = parseInt(player.value);
    if (val != 0) {
      active.push(player);
      actfact.push(parseInt(player.value));
      actname.push(capitaliseFirstLetter(player.id));
    }
  }

  table += "<table>\n";
  table += "  <tr><th>Value</th><th>Message</th></tr>\n";
  for (var i = rFrom; i <= rTo; i++) {

    var msg = "";
    for (var p = 0; p < active.length; p++) {
      if (i % actfact[p] == 0) {
        msg += actname[p];
      }
    }
    if (msg == "") {
      msg = "" + i;
    }
    table += "  <tr><td>" + i + "</td><td>" + msg + "</td></tr>\n";
  }
  table += "</table>\n";

  fizzDiv.innerHTML = table;
}
h1 {
    clear: left;
}

hr {
    clear: left;
}

label {
    display: inline-block;
    float: left;
    clear: left;
    width: 150px;
    text-align: left;
}
input {
  display: inline-block;
  float: right;
  text-align: right;
  padding-left:10px;
  width: 50px;
}
<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>FizzBuzz</title>
    <link rel="stylesheet" href="fizzbuzz.css">
    <script src="fizzbuzz.js" type="text/javascript"></script>
  </head>
  <body>
    <h1>Config</h1>
    <form id="fizzbuzz">
      <fieldset id="fizzControl" oninput="fizzing();">
        <label>Range From<input id="rangeFrom" type="number" min="1" max="100" value="1" required></label>
        <label>Range To<input id="rangeTo"   type="number" min="1" max="1024" value="100" required></label>
        <div id="players" >
          <label>Frodo<input id="frodo" type="number" min="0" max="100" value="3" required></label>
          <label>Sam<input id="sam" type="number" min="0" max="100" value="5" required></label>
          <label>Merry<input id="merry" type="number" min="0" max="100" value="0" required></label>
          <label>Pippin<input id="pippin" type="number" min="0" max="100" value="0" required></label>
          <label>Bilbo<input id="bilbo" type="number" min="0" max="100" value="0" required></label>
        </div>
      </fieldset>
    </form>
    <hr>
    <h1>Output</h1>
    <div id="fizzOut" >Change a value to get output (a snippet thing)</div>
    <script>fizzing();</script>
  </body>
</html>

Upvotes: 4

Views: 1095

Answers (6)

Claudiu
Claudiu

Reputation: 229321

Check what is actually being assigned to player:

> fizzPlayers = ["jim", "bob", "joe"]
> for (var player in fizzPlayers) {
    console.log(player);
  }
0
1
2

Iterating through an object gives you the keys. The keys of an array are the indices. In this case you are better off using a regular for loop rather than this for-each loop, since if you have assigned any other properties to the array (like fizzPlayers.barg = 40;) the for-each loop will give you those property names as well

However, if you are using jQuery, you can use $.each:

> $.each(fizzPlayers, function (index, player) { 
    console.log(index + ": " + player); 
  });
0: jim
1: bob
2: joe

Note that the callback takes two parameters: the index, and the value.

Upvotes: 4

joews
joews

Reputation: 30330

The idiomatic way to write a loop that iterates over Array elements is forEach. As comments have noted, for... in is for Object keys, not Array elements.

fizzPlayers.forEach(function(player) {
  var val = parseInt(player.value);
  if (val != 0) {
    active.push(player);
    actfact.push(parseInt(player.value));
    actname.push(capitaliseFirstLetter(player.id));
  }
});

You can Array.prototype.forEach in all modern browsers (IE9 +).

MDN docs

Upvotes: 9

Jerome Anthony
Jerome Anthony

Reputation: 8021

The for-each loop assigns the index

// prints undefined... but why?
for (var playerIndex in fizzPlayers) {
   console.log(fizzPlayers[playerIndex].value);
}

You could consider a convenience library like lo-dash for utility functions.

Upvotes: 1

200_success
200_success

Reputation: 7582

You need

for (var p in fizzPlayers) {
    console.log(fizzPlayers[p].value);
}

This is a common annoyance. An ECMAScript 6 proposal has

for (var player of fizzPlayers) {
    console.log(player.value);
}

Upvotes: 2

Christos
Christos

Reputation: 53958

A for in loop in JavaScript runs through the enumerable properties of an object. Hence, you can't convert it to a classic for loop. More formally, from the MDN:

A for...in loop only iterates over enumerable properties. Objects created from built–in constructors like Array and Object have inherited non–enumerable properties from Object.prototype and String.prototype, such as String's indexOf() method or Object's toString() method. The loop will iterate over all enumerable properties of the object itself and those the object inherits from its constructor's prototype (properties closer to the object in the prototype chain override prototypes' properties).

Upvotes: 1

Scott Hunter
Scott Hunter

Reputation: 49803

In Javascript, the for-each loop assigns the index to the control variable, so you would need to use fizzPlayers[player] instead of just player. You might not like it (I often don't), but that's how it works.

Upvotes: 3

Related Questions