Ping pong javaScript test: Unexpected list after every 5th index?

Question

So I'm learning javascript and decided to do the ping pong test challenge. Basically you it prompts you a number and displays all the numbers except "ping" for multiples of 3, "pong" for multiples of 5, and "ping-pong" for multiples of 15. I got it to work, but for some reason it displays an empty list after every 5th index. I don't know why this is, and so hopefully I can get some help with this and some explanation. Here is my code:

<!DOCTYPE html>
<html>
    <head>
        <script src="epicodus/js/jquery-1.11.2.js"></script>
        <script src="epicodus/js/ping-pong.js"></script>

        <title>Ping Pong Test</title>

    </head>

    <body>
        <ul id="list">
        </ul>
    </body>
</html>

$(document).ready(function() {
var number = parseInt(prompt("What number would you like me to ping-pong up to?"));

for(index = 1; index <= number; index += 1) {
    if (index % 15 === 0) {
        $("#list").append("<li>" + "ping-pong" + "</li>");
    } else if (index % 3 === 0) {
        $("#list").append("<li>" + "ping" + "</li>");
    } else if (index % 5 === 0) {
        $("#list").append("<li>" + "pong" + "<li>");
    } else {
        $("#list").append("<li>" + index + "</li>");
    }
}

});

Also, do for loops in javascript require a semi-colon after closing the loop?

Your closing in the if(index%5===0) section is missing its slash — ateich, Jan 17 '15 at 02:37

Anthony Forloney · Answer 1 · 2015-01-17T02:54:24.347

When your condition evaluates to true for your index % 5 === 0 condition, the code appends an li element, but forgets to end the tag with the respective </li> tag (since it is missing).

The browser will interpret the markup as two back-to-back <li> elements (opposed to one as you had intended) and will attempt to close the tags appropriately resulting in a blank item appearing.

To illustrate the problem, your above code will generate the following HTML markup:

<ul id="list">
   <li>1</li>      
   <li>2</li>      
   <li>ping</li>      
   <li>4</li>      
   <li>pong</li>   # your code forgot to close <li> tag so it's really <li>pong<li>
   <li></li>       # browser close out extra <li>, rendering as empty value
   <li>ping</li>   
   <li>7</li>
   ...
</li></ul>

By closing the <li> tag in the code block everything works as intended:

var number = parseInt(prompt("What number would you like me to ping-pong up to?"));

for(index = 1; index <= number; index += 1) {
    console.log(index);
    if (index % 15 === 0) {
        $("#list").append("<li>ping-pong</li>");
    } else if (index % 3 === 0) {
        $("#list").append("<li>ping</li>");
    } else if (index % 5 === 0) {
        $("#list").append("<li>pong</li>");   # < note the ending tag
    } else {
        $("#list").append("<li>" + index + "</li>");
    }
}

No, you are not required to use a semi-colon after your for loop.

Great answers guys! Thank you! I tried debugging before and didn't see that didn't close the li tag. Why do you guys add console.log(index)? What does that do? — Tamothy, Jan 17 '15 at 17:05
@Tamothy also, to know if you're missing `;` or other stuff you can check your code on jshint or jslint .com Or simply use a better code editor (that will highlight errors for you.) — Roko C. Buljan, Jan 17 '15 at 17:12

score 0 · Answer 2 · answered Jan 17 '15 at 05:52

var number = parseInt(prompt("What number would you like me to ping-pong up to?"));

for(index = 1; index <= number; index += 1) {
    console.log(index);
    var $li = $('<li>');
    if (index % 15 === 0) {
        $("#list").append($li.text(ping-pong));
    } else if (index % 3 === 0) {
        $("#list").append($li.text(ping));
    } else if (index % 5 === 0) {
        $("#list").append($li.text(pong));
    } else {
        $("#list").append($li.text(index));
    }
}

I find it as a best practices when appending html elements to create a jquery html object like this -> $('

') It automatically encloses your elements so you wont run into that problem.

Great answers guys! Thank you! I tried debugging before and didn't see that didn't close the li tag. Why do you guys add console.log(index)? What does that do? — Tamothy, Jan 17 '15 at 17:05
console.log(index) lets you see the value in index while debugging — Tyler Oliver, Jan 19 '15 at 04:58

Roko C. Buljan · Answer 3 · 2015-01-17T18:42:56.377

jsBin demo

^{After you fix your typo with the closing </li> tag...}

You forgot to assign index to var
When using parseInt Method add the radix parameter (10 for decimal).
Don't access all over again the same Element Selector inside a loop. It's terribly slow.
On every loop iteration you're forcing your script to search for it, instead of: creating a string, concatenate your string inside a loop, than once it's done append only once the generated HTML to your element! A user input of 20000 will give you a result in matter of seconds.
Multiple of 15 are all the numbers that are both multiple of 3 and 5.
For loop argument, i += 1 is the same as i++
$(function() { is a short way to wait for DOM is ready

Let's apply the above:

$(function() { // DOM ready shorthand

  var n = parseInt(prompt("Enter a Number"), 10);
  var li  = "";              // The string to concatenate HTML
  for(var i=1; i<=n; i++) {
    li += "<li>"+( (i%3?"":"ping") + (i%5?"":"pong") || i )+"</li>";
  }
  $("#list").html(li);      // Append only once

});

So what does (i%3?"":"ping") + (i%5?"":"pong") || i do?

(i%3?"":"ping") >> if i%3 is true (greater than 0) use "", else use "ping"
 +              >> concatenate
(i%5?"":"pong") >> if i%5 is true (greater than 0) use "", else use "pong"
 ||             >> (OR) else (if the concatenation resulted in "" [false]) than...
 i              >> use the current iteration index Number

logically from the above you can conclude that:
if i is both multiple of 3 and 5 (means multiple of 15!) you'll end up with a concatenated "pingpong".
If the concatenation returned ("") + ("") equals "" and since ""|| evaluates to false, i will be used.

Additionally if you don't like the "pingpong" string that's returned from the above logic you can parse the returned string and g-replace (global flag) all "gp" instances with "g-p" to return the desired "ping-pong" like:

$("#list").html( li.replace( /gp/g, "g-p") );

Ping pong javaScript test: Unexpected list after every 5th index?

3 Answers3