# LiveTime Object Literal

I have a website that retrieves time data from any given location on the world live.

I wanted to be able to automatically change the time and not deal with the hassle of local client times.

It's fairly simple, but since I'm not that good with JS, I'd like to know if there is anything I could improve about it?

Suggestions are welcome!

(function() {
"use strict";

var LiveTime = {
rawOffset: null,
dstOffset: null,
timeoutId: null,

setOffset: function(raw, dst) {
LiveTime.rawOffset = raw * 1000;

if (typeof dst !== "undefined") {
LiveTime.dstOffset = dst * 1000;
}
},

start: function() {
LiveTime.timeoutId = (function increment() {
var now = new Date();
var now = new Date(
now.getUTCFullYear(),
now.getUTCMonth(),
now.getUTCDate(),
now.getUTCHours(),
now.getUTCMinutes(),
now.getUTCSeconds(),
now.getUTCMilliseconds()
);

now.setTime(now.valueOf() + LiveTime.rawOffset + LiveTime.dstOffset);

var element = document.getElementById("current-time");
element.innerHTML = ("0" + now.getHours()).slice(-2) + ":"
+ ("0" + now.getMinutes()).slice(-2) + ":"
+ ("0" + now.getSeconds()).slice(-2);

LiveTime.timeoutId = setTimeout(increment, 50);
})();
},

stop: function() {
clearTimeout(LiveTime.timeoutId);
}
}

//usage example
LiveTime.start();

//simulate new location after 5 seconds
setTimeout(function() {
}, 5000);

//stop the time after 5 more seconds
setTimeout(function() {
LiveTime.stop();
}, 10000);
})();
<div id="current-time"></div>

From a once over:

• dst stands for daylight saving time, you should comment that
• offset I assume is the offset to the UTC (Greenwich time), you should comment that as well
• I am ambiguous about declaring rawOffset, dstOffset, timeoutId. I can see the value of self documenting code, but is it worth it ? Not sure. The code works fine if you delete those 3 statements.
• Consider falsy evaluations

if (typeof dst !== "undefined") {
LiveTime.dstOffset = dst * 1000;
}


compared to

if (dst) {
LiveTime.dstOffset = dst * 1000;
}


or even (a bit hacky)

LiveTime.dstOffset = dst ? dst * 1000 : 0;

• Do not declare variables twice like here:

var now = new Date();
var now = new Date(

• You can get the timestring this way:

now.toTimeString().substring(0,8);


This basically keeps only the first 8 (hh:mm:ss) characters of the time string

• You can get the UTC date value this way:

new Date((new Date()).toUTCString().slice(0,-4) )


This gets the UTC date, strips off the timezone part and re-creates the date

• document.getElementById("current-time"); <- This should be a parameter

• As @Pinoniq mentioned, ideally you should write your class/code so that you can run multiple instances.

• LiveTime is not a constructor (yet), so it should be called liveTime

This would be my counter proposal:

function liveTime(elementId, offsetSeconds, dayLightTimeSavingSeconds) {
"use strict";

var timeZoneOffset = 0,
daylightTimeSavingOffset = 0,
element = document.getElementById(elementId),
timeoutId;

if( offsetSeconds ){
setTimeOffset( offsetSeconds, dayLightTimeSavingSeconds );
}

function setTimeOffset(offsetSeconds, dayLightTimeSavingSeconds) {
//Convert to milliseconds
timeZoneOffset = offsetSeconds * 1000;
daylightTimeSavingOffset = (dayLightTimeSavingSeconds || 0) * 1000;
}

function start() {
if( timeoutId ){
}
(function increment() {
//Set the time to UTC time
var now = new Date((new Date()).toUTCString().slice(0, -4));
now.setTime(now.valueOf() + timeZoneOffset + daylightTimeSavingOffset);
//Update the HTML
element.innerHTML = now.toTimeString().substring(0, 8);
//Come back again
timeoutId = setTimeout(increment, 50);
})();
}

function stop() {
clearTimeout(timeoutId);
timeoutId = 0;
}
return {
setTimeOffset: setTimeOffset,
start: start,
stop: stop
};
}

//usage example
liveTime('current-time',-9 * 3600, 3600).start();
liveTime('current-time2', 4 * 3600 ).start();
Alaska time:<span id="current-time"></span><br>
Moscow time:<span id="current-time2"></span>

• Thanks for the review! Will digest this new information some time later. But since you've made it a constructed object, shouldn't the name be LiveTime instead of liveTime? Or is it not an object with a constructor? Oct 29 '14 at 14:34
• It is not a true constructor, though it could have been called LiveTime Oct 29 '14 at 15:27
• Anything in particular why you didn't decide to go with the "normal" constructor way? Oct 29 '14 at 15:29
• Yes, it removes the need for this all over the code and has baked in support for splitting private/public parts. Oct 29 '14 at 15:39

You state that it's fairly simple. It isn't...

# Tight coupling

Everything in your code is tightly coupled. Even the id of the element where the time is displayed is hard coded into the code.

What if I want multiple timers next to each other? Useless.

# Strange naming "convention"

You use the word offset alot. But offset to what? What is your default time? After digging around I think it is UTC?

Then there is your method that let's you change the way the time is presented: setOffset. It accepts 2 arguments. raw and dst. What the hell is raw and dst? Again I have to dig into the code and still have no idea what you are doing. I think that raw and dst have to be passed in as seconds? But what is the difference between those variables? I have no idea.

dst seems to be optional. why? No idea. In the end you always count dst and raw together everytime you use them.

# Some other strange doings

you create an IIFE (immediately invoked function expression) but you give it a name (function increnment(){})();. What this piece of code does is create function called increment. It then immediately invokes that function. The return value of that invoked function is stored in LiveTime.timeoutId. Or no, wait. The invoked function itself writes to LiveTime.timeoutId. Wow, do I need to explain more?

Not to talk about the horror of increment() actually caling itself. Inception!

# KISS: Keep It Stupid Simple

Every function of your code should do 1 thing and 1 thing only. And preferably it should solve a little problem.

# Defining the problem

The first step in solving a problem - code should always solve a problem. If it doesn't, you don't need it - is defining the problem.

I wanted to be able to automatically change the time and not deal with the hassle of local client times.

• Thanks for your review. I'm using setTimeout for the function to call itself, because if I use setInterval the execution of the method would get queued up after some time. Also, isn't the point of object literals for it to be only 1 instance (singleton)? This LiveTime object literal is not meant to have multiple instances, that's why I've tightly coupled it with my HTML code. Oct 28 '14 at 14:41