2

I`m trying to do simple timer using JavaScript. Everything alright, but setTimeout() just works instantly. How can I fix it?

var flag = 0;

function getTimerValue(){
    var val = parseInt(document.getElementById('timer').value,10);
    if (isNaN(val)) {
        val = 0;
    }
    return val;
}

function setTimerValue(newvalue){
        document.getElementById('timer').value = newvalue;
}

function runTimer(){
    if (flag == 0){
        flag = 1;
        while (flag == 1){
            x = getTimerValue()
            if (x < 1){
                setTimerValue(0);
                flag = 0;
            } else{
                setTimeout(setTimerValue(x - 1), 1000);
            }
        }
    }
}
Noqrax
  • 579
  • 1
  • 5
  • 12

2 Answers2

2

It's not working because you're invoking the setTimerValue function (which is why it is executed immediately). The setTimeout method expects a function reference, therefore one option would be to change it to:

setTimeout(function () {
  setTimerValue(x - 1);
}, 1000);

Or as Paulpro points out, you could also use:

setTimeout(setTimerValue, 1000, x - 1);

It's also worth mentioning that you could use the .bind() method as well:

setTimeout(setTimerValue.bind(null, x - 1), 1000);
Community
  • 1
  • 1
Josh Crozier
  • 202,159
  • 50
  • 343
  • 273
  • 1
    Or `setTimeout( setTimerValue, 1000, x - 1 );` which works in all the same browsers as `bind` except for IE 9 ([but is easy to fix in IE too](https://developer.mozilla.org/en-US/docs/Web/API/WindowTimers/setTimeout#IE_only_fix)). – Paul Jan 05 '16 at 02:36
  • I did like u show and it's just loops ignoring else statement. – Noqrax Jan 05 '16 at 03:25
  • @Noqrax: You seem to be assuming that `setTimeout` blocks until the timeout expires. It doesn't. It queues the function for execution in the JS event loop later on, and returns immediately. And events queued in the event loop aren't run while other JS code is executing; they wait for it to finish. So you're just queuing an infinite number of events that should decrement the same count by one, the loop never exits, and so the event loop is never able to dispatch any of them. – ShadowRanger Jan 05 '16 at 04:03
  • @ShadowRanger what I have to do then? Any solutions? – Noqrax Jan 05 '16 at 04:18
  • @Noqrax: Just posted an answer with a solution for the general case. You'll need to learn more about the JS event model to understand why this is as wrong as it is. – ShadowRanger Jan 05 '16 at 04:18
1

Your code is wrong. If the timer value is non-zero, you loop forever, constantly queuing events to decrement the same initial counter value by 1 (e.g. changing 7 to 6 over and over, but never 6 to 5), but because the dispatch code never finishes, control is never handed back to the JS event loop to allow it to process any of those events.

setTimeout isn't just "sleep the specified time and then run this function", and it doesn't block; it just schedules the call to occur later, and returns instantly. The scheduled call is only processed when the event loop has control to dispatched events; since your code loops forever, the event loop never regains control.

You need to think asynchronously in JS; as soon as you invoke an async feature, you can't rely on the results anywhere but in the callback. Try restructuring your code to perform the subsequent work in asynchronously, so you actually see the updated values:

function decrementTimerUntilZero() {
    var x = getTimerValue();
    if (x >= 1) {
        setTimerValue(x - 1);
        setTimeout(decrementTimerUntilZero, 1000); // Schedule another decrement
    } else {
        setTimerValue(0);
    }
}
if (getTimerValue() > 0) {
    setTimeout(decrementTimerUntilZero, 1000);
}

You initially schedule it for decrementing if it's positive, then each time you decrement, you schedule a new decrement if the value hasn't already reached zero.

An alternative approach (that would likely get more consistent timing, avoiding the problem of an event loop dispatch delay never being made up) would be to use setInterval:

var timerinterval = setInterval(function() {
    var x = getTimerValue();
    if (x >= 1) {
        setTimerValue(x - 1);
    } else {
        setTimerValue(0);
        clearInterval(timerinterval);
    }
}, 1000);

In both cases, you essentially give up control when your function is finished to let the event loop do more work, trusting that your callback will be invoked correctly in the future where it can resume work. JavaScript has no concept of a sleep function (by design; sleeping would block the event loop until it finished, freezing the browser UI); this is the only way to properly do work intermittently.

If you want more information, I suggest you check out What do I do if I want a JavaScript version of sleep()? if you want to get a better idea of how setTimeout works, why it isn't just a blocking sleep, etc.

Community
  • 1
  • 1
ShadowRanger
  • 108,619
  • 9
  • 124
  • 184