HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptMinor

ISO 8601 dates & times format into a more human readable format

Submitted by: @import:stackexchange-codereview··
0
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.

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 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.