current community

your communities

more stack exchange communities

Stack Exchange
tour help
stack overflow careers
Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 2 down vote favorite

I'm trying to populate the contents of an array with the HTML from 5 different div elements using a loop. I can't get the for loop to work correctly. Any ideas?

The HTML:

<div id="person0">John</div>
<div id="person1">Kathleen</div>
<div id="person2">Cynthia</div>
<div id="person3">Eric</div>
<div id="person4">Jay</div>



<div style="width: 600px; margin: auto; padding: 15px; border: 1px solid gray; font-family: sans-serif;">
        <h1>The People</h1>
        <div id="result"></div>
</div>

The Javascript:

function pageLoaded(){

  var list = "<ul>";
  var myArray = new Array();

  for(i=0; i<6; i++){
    var person = "person";
    var personNumber = "" + i.toString();
    var divId = person.concat(personNumber);
    myArray[i] = document.getElementById(divId).innerHTML;
    list +="<li>"+myArray[i]+"</li>";
  }


  list +="</ul>";

  document.getElementById("result").innerHTML=list;

}
share|improve this question
2  
I know it's not an answer, but have you looked into using jQuery? It makes JavaScript development much easier and more elegant. –  Philip Tenn Feb 24 '13 at 2:04
    
I've definitely thought about that but I'm trying to get better at JS functions so sort of doing this for practice :) –  MaiTar Feb 24 '13 at 2:05
    
good for you on practicing core JS, best wishes with that. Tossed together a page with your HTML and JavaScript, with some alerts, got it working. –  Philip Tenn Feb 24 '13 at 2:14

5 Answers 5

up vote 3 down vote accepted

Just change the for condition from i<6 to i<5.

You were running the loop one too many times, which meant on the sixth iteration when you did:

myArray[i] = document.getElementById(divId).innerHTML;

You were looking for an element with id of "person5", so getElementById() returned null which gave the error:

Uncaught TypeError: Cannot read property 'innerHTML' of null

...which stopped your script from completing.

Demo: http://jsbin.com/awubeq/1/edit

share|improve this answer
    
Thanks! Wow--it's always some silly little mistake like that. I always forget that arrays start with the index at 0. This fixed it. –  MaiTar Feb 24 '13 at 2:07
up vote 1 down vote

Here's a working solution (plus some alerts):

<html>
<head>
   <title>Test</title>
   <script>

function pageLoaded()
{
  alert("called pageLoaded");
  var list = "<ul>";
  var myArray = new Array();

  for(i=0; i<5; i++){
    var person = "person";
    var personNumber = "" + i.toString();
    var divId = person.concat(personNumber);
    myArray[i] = document.getElementById(divId).innerHTML;
    alert("myArray[i]=" + myArray[i]);
    list +="<li>"+myArray[i]+"</li>";

  }


  list +="</ul>";

  alert(list);


  document.getElementById("result").innerHTML=list;

}
</script>
</head>

<body onload="pageLoaded()">

<div id="person0">John</div>
<div id="person1">Kathleen</div>
<div id="person2">Cynthia</div>
<div id="person3">Eric</div>
<div id="person4">Jay</div>



<div style="width: 600px; margin: auto; padding: 15px; border: 1px solid gray; font-family: sans-serif;">
        <h1>The People</h1>
        <div id="result"></div>
</div>

</body>
</html>
share|improve this answer
up vote 1 down vote

If you use an iterator, you don't have to worry about knowing the size of the target of iteration.

document.getElementById('result').innerHTML =
Array.prototype.reduce.call(document.querySelectorAll('[id^=person]'),
    function (prev, elem) {
       return prev + '<li>' + elem.innerHTML + '</li>';
}, '<ul>') + '</ul>';

http://jsfiddle.net/ExplosionPIlls/Avfjs/1/

share|improve this answer
up vote 1 down vote

You are looping thru more divs than you have. If you switch it to 4, it works.

for(i=0; i<5; i++){

or 

for(i=0; i<=4; i++){
share|improve this answer
    
That will skip the last item. Needs to be <=4 or <5. –  nnnnnn Feb 24 '13 at 2:34
    
yes, you are correct - my bad. Fast w/o checking. –  james emanon Feb 24 '13 at 5:39
up vote 0 down vote

Since you are practicing, I just wanted to point out that you should reconsider the markup as this is not a case for using an id attribute but for a class. The id approach, though common, is incorrect and requires more code than necessary. Here is an example with better markup, which makes using javascript with it much more straightforward.

http://jsfiddle.net/3gpe8/

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.