Question

The code below is a sample of what I am trying to do.. on a smaller scale.This code never actually clears the interval, they just start stacking. Thanks!

<script>
var num = 0;
var counter = setInterval('start_count()',1000);

function start_count(type){
    num++;
    document.getElementById('num_div').innerHTML = num;
}
function stop_count(){
   clearInterval(counter);
   num = 0;
}
</script>
<div id="num_div"></div>
<br>
<input type="button" value="Stop" onClick="stop_count()">
<input type="button" value="Start" onClick="counter = setInterval('start_count()',1000)">

Please Help!

Was it helpful?

Solution

Please, don't pass a string to setInterval, JS functions are first-class objects, so they can be passed as arguments, assigned to variables or returned by other functions. In your example, I'd write the following:

<input type="button" value="Start" id="intervalStart"/>
<input type="button" value="Start" id="intervalStop"/>

And then, the JS:

window.onload = function()//not the best way, but just as an example
{
    var timer, num = 0, 
    numDiv = document.getElementById('num_div');//<-- only scan dom once, not on every function call
    document.getElementById('intervalStart').onclick = function(e)
    {
        this.setAttribute('disabled','disabled');//disable start btn, so it can't be clicked multiple times
        timer = setInterval(function()
        {
            numDiv.innerHTML = ++num;
        },1000);
    };
    document.getElementById('intervalStop').onclick = function(e)
    {
        document.getElementById('intervalStart').removeAttribute('disabled');//enable start again, you could do the same with disable btn, too
        clearInterval(timer);
        num = 0;
        numDiv.innerHTML = '';
    };
};

This code needs some more work, though at least it doesn't pollute the global namespace too much.
Do note that I've used a variable to reference the num_div element, that way your interval callback function doesn't have to scan the entire DOM for that element every 1000ms. It's not much, but keep in mind that every time you use a jQuery selector, or a getElement(s)By* method, JS has to traverse the DOM and look for the element. Keeping a reference to an element you'll be needing a lot just makes for a more efficient script.

OTHER TIPS

I think that this code will do what you were trying to do:

<script>
    var num = 0;
    var counter;

    function start_interval() {
        counter = setInterval(function () { start_count() }, 1000)        
    }

    function start_count() {
        num++;
        document.getElementById('num_div').innerHTML = num;
    }
    function stop_count() {
        clearInterval(counter);
        num = 0;
    }
</script>
<div id="num_div"></div>
<br>
<input type="button" value="Stop" onClick="stop_count();">
<input type="button" value="Start" onClick="start_interval();"> 
<script>
    var num = 0;
    var counter = setInterval(start_count,1000);  
    function start_count(type){     
        num++;     
        document.getElementById('num_div').innerHTML = num;
    }
    function stop_count(){    
        clearInterval(counter);    
        num = 0;
    }
</script>
<div id="num_div"></div>
<br>
<input type="button" value="Stop" onClick="stop_count()">
<input type="button" value="Start" onClick="counter = setInterval(start_count,1000)">

That's the working version. But as mentioned in the comments, one of two things should be passed to a setInterval or setTimeout:

A function which defines the code that should be executed:

setTimeout(function(){
  /* timeout code */
}, 1000);

Or a reference to the function that you want to call (exempting the quotation marks):

function myTimeoutFunction(){
  /* timeout code */
}
setTimeout(myTimeoutFunction, 1000);

Not clear exactly what the problem is. Your code works fine (aside from the comments about passing strings to setInterval). If your problem is that hitting "start" multiple times adds multiple intervals, then yeah. You can fix that by checking if the clock is already running.

Here's an example:

function startCounter() {
    if (!counter) {
        counter = setInterval(start_count,1000);
    }
}

function start_count(type){
    num++;
    document.getElementById('num_div').innerHTML = num;
}

function stop_count(){
   clearInterval(counter);
   counter = 0;
   num = 0;
}

Here's a fiddle

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top