After a few more updates we have some nicely cleaned up code.
The immediately invoked function expression provides a nicely sandboxed area for the code to run, protected from issues such as global scope pollution.
(function () {
...
}());
The variables are defined together at the top of the function.
There is potential trouble with setInterval if the initial request takes longer than a second to respond, so updating the time occurs only when the remaining time is known.
function get_time() {
...
if (!timer) {
timer = setInterval(updateCountdown, 1000);
}
}
The pad function works perfectly well, but since its behaviour is so well known is it worthwhile to condense things there somewhat using the following ternary selector?
function pad(value) {
return (value < 10 ? '0' : '') + value;
}
The rest of the code such as in updateCountdown I’ve left mostly alone, because if I start to work in there I’ll end up replacing it all with Date().toTimeString methods to automatically create formatted times.
The following isn’t used, but it’s one possible way to retrieve the remaining time from a Date object:
var date = new Date(),
remainingTime;
date.setTime(timeLeft * 1000);
remainingTime = date.toUTCString().match(/\d+:\d+:\d+/)[0],
I’ve also renamed synctime to ‘initTime’ as it kicks off the timer processes, which means that the end of the script we just have the following, to get the time left and init the timer.
getTime();
initTime();
The above leaves us now with the following code:
(function () {
'use strict';
var eventtime = 1424744309,
timeLeft = 0,
timer,
mytime;
function pad(value) {
return (value < 10 ? '0' : '') + value;
}
function updateCountdown() {
timeLeft -= 1;
var hrs = 3600,
mins = 60,
hrsLeft = Math.floor(timeLeft / hrs),
minLeft = Math.floor((timeLeft % hrs) / mins),
secLeft = Math.floor(timeLeft % mins);
document.getElementById('timer').innerHTML = "00 : " + pad(hrsLeft) + " : " + pad(minLeft) + " : " + pad(secLeft);
if (timeLeft <= 0) {
clearInterval(timer);
clearInterval(mytime);
document.getElementById('timer').innerHTML = "Ended!";
}
}
function getTime() {
document.getElementById('sync').classList.remove('hidden');
microAjax('servertime.php', function (newtime) {
document.getElementById('sync').classList.add('hidden');
timeLeft = eventtime - newtime;
});
if (!timer) {
timer = setInterval(updateCountdown, 1000);
}
}
function initTime() {
var refresh_ms = 10000;
mytime = setInterval(getTime, refresh_ms);
}
getTime();
initTime();
}());
Other improvements have been to tun the code through jslint.com to weed out some bad practices.
Other improvements can be made too so that it’s more appropriately structured and capable of handling future updates more easily, but I think that this is a good start.
Oh and by the way, are there any places that you would want to add comments? Because other than a comment block at the start to give an overall description, most other types of comments throughout the code can be easily avoided by putting things in to well-named functions, or by structuring the code so-as to make things easier to understand.
