views:

134

answers:

2

EDIT: I figured out the answer to the original YUI3 question I posted here, but it led to another one and instead of starting a new thread I thought I'd just add it here. Please scroll down for the new question (it's bolded).

Original question: I'm having some issues creating a JavaScript countdown timer inside a YUI definition, my guess is something to do with object scoping. Here's my code:

YUI({combine: true, timeout: 10000}).use("node", function (Y) {
    var timer = new function(){};
    Y.augment(timer, Y.EventTarget);
    timer.on('timer:down', function() {
        Y.log('timer down event fired', 'event');
        Y.Lang.later(1000, Y, timer_trigger());
    });
    timer.on('timer:end', function() {
        Y.log('timer end event fired', 'event');
    });

    var timer_from;

    function startTimer(seconds){ // start a coundown from seconds to 0
        timer_from = seconds;
        timer_trigger();
    }

    function timer_display(){
        var mins = Math.floor(timer_from/60);
        var secs = timer_from - mins*60;
        var secsDisp = secs;
        if(secs<10){
            secsDisp = '0' + secs;
        }
        Y.one('#timer').set('innerHTML', mins + ':' + secsDisp);
    }

    function timer_trigger(){
        Y.log('timer from is: '+timer_from);
        if(timer_from > 0){
            timer_from--;
            timer_display();
            if(timer_from > 0){
                timer.fire('timer:down');
            }
        } else {
            timer.fire('timer:end');
        }
    }

    function initializePage(){
        startTimer(900);
    }


});

The error I'm getting is that it doesn't wait the 1000ms like I'm asking it to to call timer_trigger() and Safari eventually asks me whether I want to stop running the code. When I do a few seconds after loading the page, the timer is already down to about 3, 4 minutes. I've also tried using setTimeout but that also produces the same result. Can anyone help? I would really appreciate it!

EDIT: I actually figured out a solution - this came after hours of trying tons of things, but a few more Google searches can sometimes still produce new results/answers (I found the answer on this site, actually).

So apparently my code was creating a race condition, and all I had to do to fix it is this:

setTimeout(function(){ 
    timer_trigger();
}, 1000);

I looked up race conditions, but it's unclear to me what it means in my case, and how the seemingly trivial change to my code fixed the issue I was having. So the original question in answered, but I'd like to turn this into the question that arose from the answer.

How does threading in JavaScript work and what cause my race condition, and why did the minor change in code fix the error I had?

+3  A: 

The problem is not a race condition. The reason the additional call to setTimeout "fixes" your code is because of a logic flaw in timer_trigger. Consider what happens in the case where timer_from is 1 when the function is called. Neither timer:down nor timer:end will be triggered.

function timer_trigger(){
    Y.log('timer from is: '+timer_from);
    if(timer_from > 0){      // Since timer_from is 1, the if block is entered
        timer_from--;        // timer_from is 0
        timer_display();
        if(timer_from > 0){  // This block is not entered, but it has no matching else
            timer.fire('timer:down');
        }
    } else {                 // The matching if block was entered, so this is not
        timer.fire('timer:end');
    }
}

You added this code:

setTimeout(function(){ 
    timer_trigger();
}, 1000);

This causes timer_trigger to be called once more with timer_from already set to 0, allowing the else block to be executed.

Jeremy Stein
Wow, I'm shocked I missed that... although my timer_from never got to 1, since I always stopped it around 200 or so. Luke's answer actually explains why my timer_trigger didn't wait 1000ms to fire.
hora
+2  A: 

Also note that

Y.Lang.later(1000, Y, timer_trigger());

executes timer_trigger immediately and passes the return value to Y.Lang.later. You probably meant

Y.Lang.later(1000, Y, timer_trigger);
Luke
So that would pass the function to Y.Lang.later, which would execute it 1000ms later, instead of the return value of the function?
hora
yep. timer_trigger as it is written above does not include a return statement, so by default it returns undefined. For Y.Lang.later(1000, Y, timer_trigger()), undefined is passed to Y.Lang.later as the function to execute in one second. this translates to setTimeout(function () { undefined.apply(Y); }, 1000);which is a runtime error.
Luke