snippetjavascriptMinor
ISO 8601 dates & times format into a more human readable format
Viewed 0 times
formathumanreadableintodatesmoretimes8601iso
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:
Then, you call the function, passing an object to it which contains all the specifications. The function then operates by itself.
The actual code:
```
(function (window, document, undefined) {
'use strict';
if (N
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:
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 (N
Solution
This review does likely not cover everything
Function definition
You are using two forms of function definition. I recommend using the style
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:
Nesting functions
Nesting functions can sometimes be useful. However, I think you are over-doing it in the following snippet:
Why not create a function
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
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
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
Strictly speaking, the
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.Code Snippets
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 : '';
}var relativeTime = function (spec) {
function init() {
var nodes = document.querySelectorAll(spec.node);
nodes.forEach(function (node) {
(function setRelativeTime() {nodes.forEach(setRelativeTime);Context
StackExchange Code Review Q#147492, answer score: 3
Revisions (0)
No revisions yet.