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

Simple elapsed time counter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
elapsedsimplecountertime

Problem

I've written a simple JavaScript function based off some existing code that takes a specific UNIX time string (in the snippet: 1491685200). It shows an alert if the specified time is still in the future. When that date has passed, it shows the elapsed days, hours and minutes whenever the function is called in-page.

I'm don't have much JavaScript object experience so it works just fine as-is, but I think it can be optimized further before I migrate my other stuff over to ES6.

function ElapsedTime () {
  var nTotalDiff = Math.round((new Date()).getTime() / 1000) - 1491685200;
  if (nTotalDiff >= 0) {
    var oDiff = {};
    oDiff.days = Math.floor(nTotalDiff / 86400);
    nTotalDiff -= oDiff.days * 86400;
    oDiff.hours = Math.floor(nTotalDiff / 3600);
    nTotalDiff -= oDiff.hours * 3600;
    oDiff.minutes = Math.floor(nTotalDiff / 60);
    nTotalDiff -= oDiff.minutes * 60;
    oDiff.seconds = Math.floor(nTotalDiff);
    return oDiff;
  } else {
    alert('nTotalDiff still not >=0, so must be before the specified date');
  }
}

function TimePassed() {
  oDiff = ElapsedTime();
  if(oDiff) {
    alert(oDiff.days + 'd ' + oDiff.hours + 'h ' + oDiff.minutes + 'm ');
  }
}

//   document.getElementById('counter').addEventListener('click', TimePassed, false);

Solution

Remarks to your code

  • Function names should be using camelCase.



  • Two minor formatting errors: function ElapsedTime (), if(oDiff).



  ↑    ↑

  • You mentioned ES6 in your question, so: var nTotalDiff could be declared using let and var oDiff using const.



  • You declare oDiff in TimePassed() as a global variable (you forgot var).



  • oDiff.seconds is declared, but never used.



  • (new Date()) can be written as new Date().



  • Specifying 1491685200 in human readable format would be better, I think.


Update: suggestion removed due to the problem pointed out in the comment to this answer.

  • If you leverage power of modulo operator, you can get rid of all these nTotalDiff -= oDiff.██████ * ███.



  • alert(oDiff.days + 'd ' + oDiff.hours + 'h ' + oDiff.minutes + 'm ') can be written in ES6 using template literals as alert(${oDiff.days}d ${oDiff.hours}h ${oDiff.minutes}m ), although space after m is unnecessary whatsoever.



  • Having two functions named ElapsedTime() and TimePassed() seem ambiguous.



  • Your code is not in strict mode.



My rewrite



const elapsedTime = () => {
'use strict';
const since = 1491685200000, // Saturday, 08-Apr-17 21:00:00 UTC
elapsed = (new Date().getTime() - since) / 1000;

if (elapsed >= 0) {
const diff = {};

diff.days = Math.floor(elapsed / 86400);
diff.hours = Math.floor(elapsed / 3600 % 24);
diff.minutes = Math.floor(elapsed / 60 % 60);
diff.seconds = Math.floor(elapsed % 60);

let message =
Over ${diff.days}d ${diff.hours}h ${diff.minutes}m ${diff.seconds}s.;
message = message.replace(/(?:0. )+/, '');
alert(message);
}
else {
alert('Elapsed time lesser than 0, i.e. specified datetime is still in the future.');
}
};

document.getElementById('counter').addEventListener('click', elapsedTime, false);

Check elapsed time



Remarks

  • /(?:0. )+/ matches sequences of one or more zero followed by any character and a space. It is used to change potential output that would look like 0d 0h 0m 1s to simply 1s.



  • In message I added word over, because the calculated time is in fact the least amount of time that could have passed (calculation time, Math.floor(), instantaneous nature of time specification invalidation).

Context

StackExchange Code Review Q#160200, answer score: 5

Revisions (0)

No revisions yet.