Timer in pure JavaScript

Question

I was working on a timer code in pure JavaScript and would like to know any pointers to improve:

Features

• Start/ Stop/ Reset on click of a button.
• Set limit to clock.
• Update class name for warning and error based on threshold timer.

JSFiddle

function timer() {
 var time = {
 sec: 00,
 min: 00,
 hr: 00
 };
 var finalLimit = null,
 warnLimit = null,
 errorLimit = null;
 var max = 59;
 var interval = null;
 function init(_hr, _min, _sec) {
 time["hr"] = _hr ? _hr : 0;
 time["min"] = _min ? _min : 0;
 time["sec"] = _sec ? _sec : 0;
 printAll();
 }
 function setLimit(fLimit, wLimit, eLimit) {
 finalLimit = fLimit;
 warnLimit = wLimit;
 errorLimit = eLimit;
 }
 function printAll() {
 print("sec");
 print("min");
 print("hr");
 }
 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).innerHTML = _time;
 }
 function validateTimer() {
 var className = "";
 var secs = time.sec + (time.min * 60) + (time.hr * 60 * 60);
 if (secs >= finalLimit) {
 stopTimer();
 } else if (secs >= errorLimit) {
 className = "error";
 } else if (secs >= warnLimit) {
 className = "warn";
 }
 var element = document.getElementsByTagName("span");
 document.getElementById("lblsec").className = className;
 }
 function startTimer() {
 init();
 if (interval) stopTimer();
 interval = setInterval(function() {
 update("sec");
 validateTimer();
 }, 1000);
 }
 function stopTimer() {
 window.clearInterval(interval);
 }
 function resetInterval() {
 stopTimer();
 time["sec"] = time["min"] = time["hr"] = 0;
 printAll();
 startTimer();
 }
 return {
 'start': startTimer,
 'stop': stopTimer,
 'reset': resetInterval,
 'init': init,
 'setLimit': setLimit
 }
};
var time = new timer();
function initTimer() {
 time.init(0, 0, 0);
}
function startTimer() {
 time.start();
 time.setLimit(10, 5, 8);
}
function endTimer() {
 time.stop();
}
function resetTimer() {
 time.reset();
}

span {
 border: 1px solid gray;
 padding: 5px;
 border-radius: 4px;
 background: #fff;
}
.timer {
 padding: 2px;
 margin: 10px;
}
.main {
 background: #efefef;
 padding: 5px;
 width: 200px;
 text-align: center;
}
.btn {
 -webkit-border-radius: 6;
 -moz-border-radius: 6;
 border-radius: 6px;
 color: #ffffff;
 font-size: 14px;
 background: #2980b9;
 text-decoration: none;
 transition: 0.4s;
}
.btn:hover {
 background: #3cb0fd;
 text-decoration: none;
 transition: 0.4s;
}
.warn {
 background: yellow;
}
.error {
 background: red;
}

<div class="main">
 <div class="timer"> 
 <span id="lblhr">00</span>
 : <span id="lblmin">00</span>
 : <span id="lblsec">00</span>
 </div>
 <button class="btn" onclick="startTimer()">Start</button>
 <button class="btn" onclick="endTimer()">Stop</button>
 <button class="btn" onclick="resetTimer()">Reset</button>
</div>

• Vogel612: I have rolled back the last edit. Please see what you may and may not do after receiving answers .
• Rajesh Dixit: @Vogel612 Thanks. I'll keep that in mind. :-)

Answer 1

It looks really good! Here are a few points;

---

I don't like the arguments in your setLimit and init functions, especially because some of them are optional.

I would prefer an Object argument in this case.

function setLimit(limits) {
 finalLimit = limits.final;
 warnLimit = limits.warn;
 errorLimit = limits.error;
}
this.setLimit({
 final: 10,
 warn: 5,
 error: 8
});

This makes it obvious at call time which argument is which.

---

Your var declarations are inconsistent here:

var finalLimit = null,
 warnLimit = null,
 errorLimit = null;
var max = 59;
var interval = null;

Pick either to comma separate them or not. Not both.

---

In a couple of places you use == instead of ===. Generally we stick to === for reasons.

---

You CSS selectors are very generic. This is generally a bad thing as they will easily collide with other styles the moment you integrate it into a real page.

I favour BEM syntax, which would make it look something like this;

<div class="timer">
 <div class="timer__labels"> 
 <span id="timer__hours-label">00</span>
 : <span id="timer__minutes-label">00</span>
 : <span id="timer__seconds-label">00</span>
 </div>
 <button class="timer__btn js-start-timer">Start</button>
 <button class="timer__btn js-end-timer">Stop</button>
 <button class="timer__btn js-reset-timer">Reset</button>
</div>

Note I've also removed your onclick handlers in favour of interaction specific classes. I prefix my classnames with js- if they are used solely for JavaScript selectors. This helps decouple the CSS and JavaScript.

In your code you then attach the handlers;

document.querySelectorAll('.js-start-timer')[0]
 .addEventListener('click', startTimer);

• Rajesh Dixit: Thanks! I would incorporate these points. Also I knew what == and === do but didn't know its not a good practice to use ==. Will keep this in mind.
• Jivings: @Rajesh It's one of those things where sometimes you will need ==, but generally it's a source of very annoying bugs.
• Rajesh Dixit: Is it bad to use onclick or other events? I usually find them better. You can read HTML and you know where to look. Problem with event handlers that I feel, when multiple developers are involved, they add multiple event handlers to same element, and this complicates things. Though I'll try to incorporate that as well
• Jivings: @Rajesh Yeah, generally you want to keep your JavaScript code separate from your HTML. However, the js- classes allow me to see exactly what bit of JavaScript will be interacting with that element.
• Jivings: @Rajesh Here's a good article about decoupling