I am wondering why this code does not work. When I put it inside the onload function, it does not work, and I am not sure why. I have a lot of other code in the onload function, but I have deleted it for this example. Anyway the other code works fine, only this part does not.

window.onload = function() {
var i, timer, divide;
i = 0;
divide = 100;

function start() {
    timer = self.setInterval("increment()", (1000 / divide)); 
}

function increment() {
    i++;
    document.getElementById("timer_out").innerHTML = (i / divide);
}

function stop() {
    clearInterval(timer);
    timer = null;
}

function reset() {
    stop();
    i = 0;
    document.getElementById("timer_out").innerHTML = (i / divide);
}

}

<div>
    <span id="timer_out">0</span>
    </br>
    <input type="button" value="Start" onclick="start()"/>
    <input type="button" value="Stop" onclick="stop()"/>
    <input type="button" value="Reset" onclick="reset()"/>
</div>

I am wondering why this code does not work. When I put it inside the onload function, it does not work, and I am not sure why. I have a lot of other code in the onload function, but I have deleted it for this example. Anyway the other code works fine, only this part does not.

window.onload = function() {
var i, timer, divide;
i = 0;
divide = 100;

function start() {
    timer = self.setInterval("increment()", (1000 / divide)); 
}

function increment() {
    i++;
    document.getElementById("timer_out").innerHTML = (i / divide);
}

function stop() {
    clearInterval(timer);
    timer = null;
}

function reset() {
    stop();
    i = 0;
    document.getElementById("timer_out").innerHTML = (i / divide);
}

}

<div>
    <span id="timer_out">0</span>
    </br>
    <input type="button" value="Start" onclick="start()"/>
    <input type="button" value="Stop" onclick="stop()"/>
    <input type="button" value="Reset" onclick="reset()"/>
</div>

• javascript
• jquery
• css

• 4 Have you checked your console? (Hit F12 to open it). Your answer lies in there. Basically, your functions need to be global to access them in your onclick handlers. A better approach would be to use .addEventListener. – Mike Cluck Commented Feb 16, 2016 at 16:50
• @MikeC i am confused about this, what do u mean? – Blindeagle Commented Feb 16, 2016 at 16:52
• 1 Basically what @MikeC trying to say is, your every buttons' onclick event is unable to call its function since you wrap their function inside onload event. The thing is, your buttons load before its function declared. – choz Commented Feb 16, 2016 at 16:53

3 Answers 3

You need to define your functions and variables outside the onload scope, otherwise they aren't accessible.

var i, timer, divide;
function start() {
    if(!timer) {
        timer = setInterval("increment()", (1000 / divide)); 
    }
}

function increment() {
    i++;
    document.getElementById("timer_out").innerHTML = (i / divide);
}

function stop() {
    clearInterval(timer);
    timer = null;
}

function reset() {
    stop();
    i = 0;
    document.getElementById("timer_out").innerHTML = (i / divide);
}
window.onload = function() {
    i = 0;
    divide = 100;
}

As mentioned in ments, you also have a bug that will make it impossible to stop the timer if you click start more than once. This is because you are creating two (or more) timers but only have a reference to the last one. To fix this, a simple if statement checking if a timer exists should suffice. Code above has been updated to show this, and here is an example of it running: http://jsfiddle/qPvT2/40/

Problems:

1. Functions referened from onxyz attributes must be globals. Your functions aren't globals, because they're declared inside your onload handler. Some people will tell you to make them globals, but there's a better answer.

2. onload is almost certainly much, much later than you'd really like this code to run. It waits until all images have fully-downloaded, for instance.

3. Using strings with setTimeout/setInterval is usually not a good idea.

4. </br> is invalid HTML. In HTML, a br element is written <br>. You can also write it <br/> or <br /> if you like, but the / is meaningless in HTML. (It has meaning in XHTML.)

5. There's no need to prefix setTiemout/setInterval with self.. It works, because there's a default self global, but it's unnecessary.

6. If you click start twice, you'll overwrite the previous timer's handle and not be able to stop it. We need to call stop before starting.

See ments for how I've addressed those below.

Without jQuery (because you tagged it, but don't seem to be using it)

// This script is at the end of the HTML, just beofre the closing
// </body> tag, ands o all the elements defined by the HTML above it
// will be there and ready for us to handle. No need for `onload`.
// But we'll use a scoping function to avoid creating globals.
(function() {
  var i, timer, divide;

  // Hook up our buttons.
  // For a very simple thing like this, we can just assign to 
  // their onclick properties, but for anything more plicated
  // I'd use addEventListener (falling back to attachEvent if
  // the browser doesn't have addEventListener, to support obsolete
  // IE like IE8), but that's overkill for this small example.
  document.querySelector("input[value=Start]").onclick = start;
  document.querySelector("input[value=Stop]").onclick = stop;
  document.querySelector("input[value=Reset]").onclick = reset;
  
  i = 0;
  divide = 100;
  timer = 0; // A value we can safely pass to clearInterval
             // if we never set anything else

  function start() {
    // Make sure to stop any previous timer
    stop();
    
    // Note no quotes or (), we're passing a reference to the
    // increment function into `setInterval`.
    // Also no need for `self.`
    timer = setInterval(increment, (1000 / divide));
  }

  function increment() {
    i++;
    document.getElementById("timer_out").innerHTML = (i / divide);
  }

  function stop() {
    clearInterval(timer);
    timer = null;
  }

  function reset() {
    stop();
    i = 0;
    document.getElementById("timer_out").innerHTML = (i / divide);
  }

})();

<div>
  <span id="timer_out">0</span>
  <br>
  <input type="button" value="Start" />
  <input type="button" value="Stop" />
  <input type="button" value="Reset" />
</div>

For more about addEventListener/attachEvent and a function you can use if you need to support obsolete browsers, see this answer.

With jQuery (because you did tag it)

// This script is at the end of the HTML, just beofre the closing
// </body> tag, ands o all the elements defined by the HTML above it
// will be there and ready for us to handle. No need for `onload`.
// But we'll use a scoping function to avoid creating globals.
(function() {
  var i, timer, divide;

  // Hook up our buttons.
  $("input[value=Start]").on("click", start);
  $("input[value=Stop]").on("click", stop);
  $("input[value=Reset]").on("click", reset);
  
  i = 0;
  divide = 100;
  timer = 0; // A value we can safely pass to clearInterval
             // if we never set anything else

  function start() {
    // Make sure to stop any previous timer
    stop();
    
    // Note no quotes or (), we're passing a reference to the
    // increment function into `setInterval`.
    // Also no need for `self.`
    timer = setInterval(increment, (1000 / divide));
  }

  function increment() {
    i++;
    $("#timer_out").text(i / divide);
  }

  function stop() {
    clearInterval(timer);
    timer = null;
  }

  function reset() {
    stop();
    i = 0;
    $("#timer_out").text(i / divide);
  }

})();

<div>
  <span id="timer_out">0</span>
  <br>
  <input type="button" value="Start" />
  <input type="button" value="Stop" />
  <input type="button" value="Reset" />
</div>
<script src="https://ajax.googleapis./ajax/libs/jquery/1.11.1/jquery.min.js"></script>

Instead of writing your functions in window.onload.. Just write them outside of it... As you are writing them inside the window.onload function, they are not accessible...just write them outside and your code will run properly