0

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!

Greg Alexander
  • 1,192
  • 3
  • 12
  • 25
  • 5
    **Never** pass a string to `setInterval()` or `setTimeout()`. Doing so is as bad as using `eval()` and it results in unreadable and possibly insecure code as soon as you use variables since you need to insert them into the string instead of passing the actual variable. The proper solution is `setInterval(function() { /* your code *) }, msecs);`. The same applies to `setTimeout()`. If you just want to call a single function without any arguments, you can also pass the function name directly: `setInterval(someFunction, msecs);` (note that there are **no** `()` behind the function name) – ThiefMaster Nov 27 '12 at 16:40
  • 2
    `setInterval` should a reference a function or a literal function, and not the name of a function in a string. try either `setInterval(start_count, 1000)` or `setInterval(function(){ start_count(); }, 1000)` when you assign it. – Brad Christie Nov 27 '12 at 16:41
  • 2
    Have you tried adding `clearInterval(counter);` to the start of the `onclick` attribute of the start button? – Anthony Grist Nov 27 '12 at 16:42

4 Answers4

4

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.

Elias Van Ootegem
  • 74,482
  • 9
  • 111
  • 149
1

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();"> 
Josep
  • 12,926
  • 2
  • 42
  • 45
0
<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);
Brad Christie
  • 100,477
  • 16
  • 156
  • 200
0

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

Matt Burland
  • 44,552
  • 18
  • 99
  • 171