Problem
I wrote from the ground up this small piece of code which transforms dates and times formatted in ISO 8601 format into a more human readable format, client-side. The main goal is that you can create multiple and fully customizable instances that differ from each other for any given node living inside the DOM tree. It updates the time live if specified. I know there are some libraries out there that does it already, but they are way too big and rely on external frameworks. I intend to make it available for all on Github when it is ready.
As a beginner, I feel like I might have over complicated some stuff and that my code seems like a mess (spaghetti). I’ve been given the advice to write the code first (make it work), and then refactor. However, I am having issues with the refactor part, as I don’t know where to start. Hence, this is why I am seeking for some advice/help in producing more efficient and understandable code.
First, you have to create some HTML element(s) with a custom selector and a custom attribute which holds the data generated by your application using a supported ISO 8601 format:
<time class="relative-time" datetime="2016-05-03T02:41:44.741Z"></time>
Then, you call the function, passing an object to it which contains all the specifications. The function then operates by itself.
relativeTime({
node: ".relative-time",
data: "datetime",
prefix: {
past: "",
future: ""
},
suffix: {
past: "ago",
future: "from now"
},
second: {
singular: "one second",
plural: "%d seconds"
},
allowTimesUpdate: true,
minute: {
singular: "one minute",
plural: "%d minutes"
},
hour: {
singular: "an hour",
plural: "%d hours"
},
day: {
singular: "a day",
plural: "%d days"
},
week: {
singular: "a week",
plural: "%d weeks"
},
month: {
singular: "a month",
plural: "%d months"
},
year: {
singular: "a year",
plural: "%d years"
}
});
The actual code:
(function (window, document, undefined) {
'use strict';
if (NodeList.prototype.forEach === undefined) {
NodeList.prototype.forEach = function (cb) {
[].forEach.call(this, cb);
};
}
function multiplyArrayItems(arr) {
var sum = 1;
for (var i = 0; i < arr.length; i++) {
sum = sum * arr[i];
}
return sum;
}
function updateRelativeTime(fn, rate) {
window.setTimeout(function () {
if (window.requestAnimationFrame) {
window.requestAnimationFrame(fn);
} else {
fn();
}
}, rate);
}
var relativeTime = function (spec) {
function init() {
var nodes = document.querySelectorAll(spec.node);
nodes.forEach(function (node) {
(function setRelativeTime() {
var now = new Date().getTime();
var data = new Date(node.getAttribute(spec.data)).getTime();
var distance = (now - data) / 1000;
var prefix = distance > 0 ? spec.prefix && spec.prefix.past ? spec.prefix.past : '' : spec.prefix && spec.prefix.future ? spec.prefix.future : '';
var suffix = distance > 0 ? spec.suffix && spec.suffix.past ? spec.suffix.past : '' : spec.suffix && spec.suffix.future ? spec.suffix.future : '';
distance = Math.abs(distance);
var times = [60, 60, 24, 7, 4.35, 12];
var units = [spec.second, spec.minute, spec.hour, spec.day, spec.week, spec.month, spec.year];
for (var i = 0; distance >= times[i] && i < times.length; i++) {
distance /= times[i];
}
distance = Math.floor(distance);
var unit = distance <= 1 ? units[i].singular : units[i].plural;
var relativeTime = unit.includes('%d') ? unit.replace('%d', distance) : unit;
var output = prefix + ' ' + relativeTime + ' ' + suffix;
node.textContent = output.trim();
// Best way to reduce performance hit, at the cost of accuracy.
// Attach a custom update rate according to the node's time unit
// instead of refreshing every X amount of milliseconds
var updateRate = multiplyArrayItems(times.splice(0, i)) * 1000;
// Update will only occur when the user browser's tab is active and
// will only attach the custom update rate if its value is
// less than or equal to an hour
if (spec.allowTimesUpdate === true && updateRate <= 3600000) {
// Check if the node has not been removed from the DOM
if (node.parentNode) {
updateRelativeTime(setRelativeTime, updateRate);
}
}
}());
});
}
if (document.readyState === 'interactive' || document.readyState === 'complete') {
init();
} else {
document.addEventListener('DOMContentLoaded', init, false);
}
};
// Export as global
window.relativeTime = relativeTime;
}(window, document));
Solution
This review does likely not cover everything
Function definition
You are using two forms of function definition. I recommend using the style function relativeTime(spec) { .. } for that function, instead of the current notation with var. You can still export it with window.relativeTime = relativeTime;.
Ternary operator
You are using a chain of ternary operators to determine the right prefix and suffix. This makes it unreadable. Instead, split it up with a few if-statements:
var prefix;
var suffix;
if (distance > 0) {
prefix = spec.prefix && spec.prefix.future ? spec.prefix.future : '';
suffix = spec.suffix && spec.suffix.future ? spec.suffix.future : '';
} else {
prefix = spec.prefix && spec.prefix.past ? spec.prefix.past : '';
suffix = spec.suffix && spec.suffix.past ? spec.suffix.past : '';
}
Nesting functions
Nesting functions can sometimes be useful. However, I think you are over-doing it in the following snippet:
var relativeTime = function (spec) {
function init() {
var nodes = document.querySelectorAll(spec.node);
nodes.forEach(function (node) {
(function setRelativeTime() {
Why not create a function setRelativeTime(node) and do the following?
nodes.forEach(setRelativeTime);
This makes the code a lot more readable.
Local data, namespace and OO
You are currently dropping your function in the global namespace. I would suggest creating a local namespace for your small library, and putting relativeTime under that. This is mainly to prevent accidental collisions with other libraries, and accidental collisions with possible future functionality of javascript itself.
You are currently fixing the “spec” of a number of nodes by calling a function, and creating functions in that function to fix the context. You could instead create a proper class, and create instances. You would end up with a new RelativeTime(spec); call probably.
Additionally, it would be easier to figure out which parts are meant to be the public interface, and which functions are “private”.
Naming of functions
Having a function updateRelativeTime, relativeTime and setRelativeTime is more than confusing. Find better names for these functions.
Strictly speaking, the sum in multiplyArrayItems is actually a product. sum is what you get when doing addition.