Confusion with simple for loop - javascript

Question

I've done FizzBuzz several times but never had this problem. Perhaps, it is something fundamental about for-loops that I don't understand. For some reason, the code below runs 10x longer than it should (well, than I think it should). If the user enters 20, it runs to 200. I fixed the problem by setting i = 0; i < num and then printing i+1 to my div, but I still don't understand why the original code does not work as expected. And while I'm at it, I might as well admit that I still can't set up JSFiddle properly. http://jsfiddle.net/nngrey/hA4pg/ (This does not run at all.) So any thoughts on that would also be appreciated. Thanks!

<head>
  <title>Fizz Buzz</title>
  <script>
    function fizzbuzz(){
      var num = prompt("Please enter a number between 1 and 100: ");
      for(var i=1; i<num+1; i++){
        if (i%3===0 && i%5===0){
          document.getElementById("div1").innerHTML = div1.innerHTML+"<p>Fizz Buzz</p>";
        }else if (i%3===0){
          document.getElementById("div1").innerHTML = div1.innerHTML+"<p>Fizz</p>";
        }else if (i%5===0){
          document.getElementById("div1").innerHTML = div1.innerHTML+"<p>Buzz</p>";
        }else{
          document.getElementById("div1").innerHTML = div1.innerHTML+"<p>"+i+"</p>";
        }
      }
    }
  </script>
</head>

<body onLoad = "fizzbuzz()">
  <div id = "div1">
    <h1>Fizz Buzz</h1>
  </div> 
</body>

to fix your fiddle put fizzbuzz(); at the bottom of the script window and take it out the body tag — Snowburnt, Oct 25 '13 at 04:08
If you choose "no wrap," on the left hand menu of jsfiddle, your code will run. The problem is that jsfiddle is adding all the javascript into the onLoad. You are calling it from onLoad in your HTML, so the function needs to be declared in or in . — foobarbecue, Oct 25 '13 at 04:08
Where are you declaring the `div1` variable? If this works for you then you're running in IE which is the only browser that auto-creates global variables based on element's ids. Don't depend on it. Always use `getElementById`. Also, you really need to do `getElementById` once and save a reference to the DOM element in a variable. Named `div1` perhaps? — slebetman, Oct 25 '13 at 04:09
@slebetman: Chrome has done this too for a long time, and now Firefox as well. — Blue Skies, Oct 25 '13 at 04:12
foobarbeque - You are right! I tried going through those options but I must have been too impatient. I probably did not notice it was still processing. JSFiddle seems to be running slow tonight. Snowburnt - I will try your suggestion too. — , Oct 25 '13 at 04:24
@slebetman I will update my code so that I use a variable for the DOM element. So would that be: var div1 = document.getElementByID("div1").innerHTML ? And then I would use it on each side of my statements: div1=div1+"
Fizz Buzz
"; ? I am not following your advice about declaring the div1 variable. I'm creating an id in my html but I don't think that's what you mean. — , Oct 25 '13 at 04:36
@slebetman - this seems to work ... var div1 = document.getElementById("div1"); div1.innerHTML = div1.innerHTML+"
Fizz Buzz
"; Better? — , Oct 25 '13 at 04:46

score 4 · Accepted Answer · answered Oct 25 '13 at 04:06

4

In your code, prompt() returns a string. Javascript will evaluate this line:

      for(var i=1; i<num+1; i++){

with num as a string. i.e num+1 becomes "20"+"1" (note the quotes) which is "201". the comparison is then evaluated numerically, so your loop runs ten times linger than it should.

In your revised version i < num is evaluated numerically, so the loop runs for the correct period.

You can force num to be a number like this:

      var num = Number(prompt("Please enter a number between 1 and 100: "));

num is now a number, so 20 + 1 = 21 (note - no quotes) and both versions of your loop should operate correctly

answered Oct 25 '13 at 04:06

So num+1 becomes the string "201" and the string "201" is converted to the int 201 when it is compared to i the first time? Is num+1 executed before the first iteration of the for-loop or immediately before it is compared to i? – Oct 25 '13 at 04:20
@Nathan I don't know for sure when the test is evaluated. In this instance it could be evaluated before the loop, but code inside the loop could theoretically change the condition, so I suspect the test is completely re-evaluated on every iteration. – Oct 25 '13 at 06:14

score 0 · Answer 2 · answered Oct 25 '13 at 04:07

0

You need to do:

var num = parseInt(prompt("Please enter a number between 1 and 100: "), 0);

prompt returns a string, so if you enter 20, num+1 is the string "201", not the number 21.

answered Oct 25 '13 at 04:07

Barmar

741,623
53
500
612

loxxy · Answer 3 · 2013-10-25T04:47:02.627

0

The prompt() returns a string.

Simple use +prompt() instead. That should make it a number. Updated code demo.

edited Oct 25 '13 at 04:47

answered Oct 25 '13 at 04:08

loxxy

12,990
2
25
56

That's handy - thanks. So "+" or "Number" or "parseInt" would all work. I don't suppose one is vastly better than the other? "+" saves on keystrokes! ;-) – Oct 25 '13 at 04:38
Actually there are quite a few differences than just saving keystrokes... Have a look: http://stackoverflow.com/questions/17106681/parseint-vs-unary-plus-when-to-use-which – loxxy Oct 25 '13 at 04:44
Ah...looks like "+" and "Number" will both preserve the decimal spaces in a float. Maybe I should used parseInt for my project to correct for non-int submissions. – Oct 25 '13 at 04:54

score 0 · Answer 4 · answered Oct 25 '13 at 04:08

0

here num is a string you have to use parseInt to convert it to int

for(var i=1; i<parseInt(num)+1; i++){
}

answered Oct 25 '13 at 04:08

Vinay Pratap Singh Bhadauria

9,829
3
28
49

Confusion with simple for loop - javascript

4 Answers4