Simple JavaScript timer function

Question

I've written a timer to start when the page loads, and to stop when the user clicks a "finished" button:

<br><button id = "finish" onclick = "stopTimer();">Done</button>

My code works fine, but I was just wondering if there's a simpler/easier/faster way to implement a timer in JavaScript.

//Stop timer function
function stopTimer() {
    clearInterval(time);
}

//Start timer function
var secTime = 0,
      minTime = 0,
      hourTime = 0;

var time =  setInterval(function(){
    var maxSec = 59,
        maxMin = 59,
        maxHour = 59;
if(secTime > maxSec){
      minTime++;
      if(minTime > maxMin){
        hourTime++;
        if(hourTime > maxHour){
          hourTime = 0;
          minTime = 0;
          secTime = 0;
        }
        minTime = 0
      }
      secTime = 0;
    }    
        var newSec = (secTime.toString().length == 1) ? '0' + secTime : secTime,
            newMin = (minTime.toString().length == 1) ? '0' + minTime : minTime,
            newHour = (hourTime.toString().length == 1) ? '0' + hourTime : hourTime;

        document.getElementById('timer').innerHTML = newHour + ':' + newMin + ':' + newSec;

    secTime++;    
}, 1000);

Answer 1

A minor thing, but this piece of code can be simplified:

if (secTime > maxSec) {
    minTime++;
    if (minTime > maxMin) {
        hourTime++;
        if (hourTime > maxHour) {
            hourTime = 0;
            minTime = 0;
            secTime = 0;
        }
        minTime = 0
    }
    secTime = 0;
}

By removing the duplicated resetting to 0, like this:

if (secTime > maxSec) {
    minTime++;
    if (minTime > maxMin) {
        hourTime++;
        if (hourTime > maxHour) {
            hourTime = 0;
        }
        minTime = 0
    }
    secTime = 0;
}    

Another minor thing, instead of the repeated logic like (x.toString().length == 1) ? '0' + x : x for seconds, minutes and hours, it would be better to extract this logic to a helper method to avoid code duplication.

Answer 2

Given the nature of the snippet, I'll just overlook the fact that you're using inline JS on your elements, and your script is entirely in the global namespace. Normally I'd recommend using something like addEventListener for your click events, and enclose your code in an IIFE.

Now to get that big elephant in the room, timers in JS are inaccurate. The delay in timers only guarantee minimum time until execution, but not exact time of execution. Your time will drift. What I would suggest is to take the time during page load. Then on set intervals, you get the current time, subtract the current time from the start time to get the time elapsed, and from that, compute your hours, minutes and seconds.

Now for the rendering, it doesn't matter how often you actually print to the page as long as you are using the time elapsed. Also, I would suggest putting the reference to the DOM element in a variable outside the interval. That way, you won't be fetching the element from the DOM every time (which is slow).

Your padding logic is also repeated. You could just put that in a function and call it when needed. You can use slice with a negative index to slice from the right. Also, since we're operating in milliseconds (because of Date.now()), and there's some division, your numbers will have decimals. To eliminate them, you can use | 0 to clip them off. It's somewhat the same as Math.floor.

Given that review, your code should look something like:

var startTime = Date.now();
var second = 1000;
var minute = second * 60;
var hour = minute * 60;
var container = document.getElementById('timer');

function stopTimer() {
    clearInterval(timer);
}

function pad(n){
  return ('00' + n).slice(-2);
}

var timer = setInterval(function(){
  var currentTime = Date.now();
  var difference = currentTime - startTime;

  var hours = pad((difference / hour) | 0);
  var minutes = pad(((difference % hour) / minute) | 0);
  var seconds = pad(((difference % minute) / second) | 0);

  container.innerHTML = hours + ':' + minutes + ':' + seconds;

// This only represents time between renders. Actual time rendered is based
// on the elapsed time calculated above.
}, 250);

<div id="timer"></div>
<button type="button" onclick="stopTimer()">Stop</button>

Answer 3

I don't know if this is better, but I took advice from @janos about removing code redundancy. I also tried to make it as a class. Please also offer any improvements.

function timer(){
    var time = {
        sec:00,
        min:00,
        hr:00
    }
    var max = 59;
    var interval = null;

    function update(str){
        time[str]++;
        time[str] = time[str]%60;
        if(time[str] == 0){
            str == "sec"? update("min"):update("hr");
        }
        print(str);
    }

    function print(str){
        var _time = time[str].toString().length == 1?"0" + time[str]:time[str];
        document.getElementById("lbl"+str).innerText = _time;
    }

    function initInterval(){
        interval = setInterval(function(){
            update("sec");
        },1000);
    }
    
    return {'initInterval': initInterval}
};

(function(){
	var time = new timer().initInterval();
})();

<div>
    <span id="lblhr">00</span>
    : <span id="lblmin">00</span>
    : <span id="lblsec">00</span>
</div>