Page is a not externally linkable
Fotiman - 1:47 pm on May 4, 2012 (gmt 0)
Welcome to WebmasterWorld, both of you. :)
1. Kudos to cybernoob for using an HTML5 doctype. :)
2. You don't need the type attribute on the script tag, so <script type="text/javascript"> could be shortened to just <script>
3. Instead of time=time-1; you can do either:
time -= 1;
time--; // I prefer this one
4. Scripts are best included at the end of your document just before the closing </body> tag.
5. You've got a minor case of divitis. ;) By that, I just mean your HTML markup is using a lot of non-semantic div elements, and there may be some semantic elements that make more sense.
6. You should avoid using inline event handlers (don't include onclick attributes within your HTML markup, instead attach the event handlers from the script itself).
Now, on to your question...
Lets step through the code.
1. User presses the "play" button, and startClock() is called.
2. startClock creates a local (this is important) variable t to store the result of setInterval, which is a timer id. However, as shingokko pointed out, your code is calling the clock function immediately, and it's the return value of that call to clock which will be called by setInterval. This can be corrected by wrapping the call to clock in an anonymous function definition. Basically, the first parameter to setInterval should be a function reference, and by including parenthesis after the function you are calling that function immediately. So, you could do this:
var t = setInterval(function() { clock(time, play);}, 1000);
When the interval happened, it would call the anonymous function, which in turn would call the clock function, passing in the global time and play variables as parameters. However, we need to look at the clock function...
Your clock function is attempting to modify the global variable time and play. However, you've defined the parameter variables of clock to have the same variable names as the globals, which means that your clock function will not be able to modify the global variables, and will instead only be operating on the local variables. So, why are we passing in those values? The local scope of those variables are preventing you from modifying the global ones, so lets get rid of them and change your function to this:
function clock() {
if (time > 0) {
time--;
document.getElementById("time").innerHTML = time;
return time;
}
else{
clearInterval(t);
play = false;
return;
}
}
Ok, so clock no longer needs to take any parameters. This also means that we can modify our setInterval call from this:
var t = setInterval(function() { clock(time, play);}, 1000);
to this:
var t = setInterval(clock, 1000);
Note, no parenthesis... we're passing the clock function reference to setInterval.
Ok, back in the clock function...
clearInterval(t);
Where was t defined? It was defined in the startClock function, outside of the scope of this clock function. In other words, t here is undefined. In order change that, t would need to be a global variable, which means removing the "var" from "var t = setInterval(clock, 1000);" in startClock, and preferably declaring var t; along with your global time and play variables.
One last note. Your clock function has 2 return lines, one that does return time, the other just returns. This function doesn't need to return anything, since nothing is acting on the return value. I would remove both of those lines to avoid any confusion.
Hope that helps.