2
\$\begingroup\$

I am creating a simple support ticket system. I thought It would be a nice little feature to show the current date and time. Since I want to show the current time I thought JavaScript would be best versus using PHP. I have looked at different questions posted by other user about the topic and created the following snippet. My question is if this is a efficient way to get and display the date and time using JavaScript date object. I also thought is would be appropriate to use the <time> tag. I also ask if I have used it in the correct form.

JavaScript and HTML

var monthName = new Array('January', 'Febuary', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December');
var hourap = new Array(12, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11);
function showTime(){
    var dateObj = new Date();
    var day = dateObj.getDate(), month = dateObj.getMonth(), year = dateObj.getFullYear(), hour = dateObj.getHours(), minutes = (dateObj.getMinutes()<=9?'0'+dateObj.getMinutes():dateObj.getMinutes());
    var string  = monthName[month]+
    ' '+day+
    ', '+year+
    '<br />'+hourap[hour]+
    ':'+minutes+
    ' '+(hour<=11?'am':'pm');
    var timeDiv = document.getElementById('time');
    if(timeDiv !== null) {
        timeDiv.innerHTML = string;
        timeDiv.setAttribute('datetime',year+'-'+(month+1<=9?'0'+(month+1):month+1)+'-'+day+' '+hour+':'+minutes);

    };
};
setInterval(showTime,1000);
<time id="time"></time>

HTML output

<time id="time" datetime="2015-01-30 22:00">January 30, 2015<br>10:00 pm</time>

Any suggestions or comments would be appreciated. Please note that I wish to script it using pure JavaScript and not jQuery.

\$\endgroup\$
2
\$\begingroup\$

Semantic markup is less important for dynamically generated HTML than it is for server-generated HTML, but it is a laudable goal nonetheless. If you're going to produce a datetime attribute for machine consumption, then I suggest using T as the delimiter between the date and time, as recommended by ISO 8601.

Since you frequently need to zero-pad a number, you should define a function for it.

For clarity, you should avoid excessively long lines of code. In addition, I think it would be a good idea to define separate functions for each of the datetime formats.

I would eliminate the hourap array, since it's not that difficult to achieve the same effect programmatically.

The timeDiv variable refers to a <time> element, not a <div>, and should therefore be renamed.

The semicolon after the final if block is unusual.

var MONTH_NAME = ['January', 'Febuary', 'March', 'April', 'May', 'June',
                  'July', 'August', 'September', 'October', 'November', 'December'];
function showTime() {
    function twoDigit(n) {
        return ('0' + n).slice(-2);
    }
    function iso8601(date) {
        return date.getFullYear() +
               '-' + twoDigit(1 + date.getMonth()) +
               '-' + twoDigit(date.getDate()) +
               'T' + twoDigit(date.getHours()) +
               ':' + twoDigit(date.getMinutes());
    }
    function en_US(date) {
        var h = date.getHours() % 12;
        return MONTH_NAME[date.getMonth()] +
               ' '  + date.getDate() +
               ', ' + date.getFullYear() +
               '<br />' + (h == 0 ? 12 : h) +
               ':'  + twoDigit(date.getMinutes()) +
               ' ' + (date.getHours() < 12 ? 'am' : 'pm');
    }
    
    var timeEl = document.getElementById('time');
    if (timeEl !== null) {
        var now = new Date();
        timeEl.innerHTML = en_US(now);
        timeDiv.setAttribute('datetime', iso8601(now));
    }
};
setInterval(showTime, 1000);
<time id="time"></time>

\$\endgroup\$
1
\$\begingroup\$

In my opinion the showTime() function is too complex and it should be split into several well defined functions each with single responsibility.

My suggestions:

  • The value for time interval was placed somewhere in the code and if you need to change it you should scroll or search by keyword. If you extract it in separate variable and place it on top it will be easier to configure.
  • The name showTime is too common and doesn't give me idea what exactly will show me, so I think that showCurrentTime is more precise.
  • The code in the current implementation of show function could be separated into 3 logical parts:

    1. With getCurrentTimestamp() we receive timestamp object with respective properties.
    2. buildTimestampString() use created timestamp object instead multiple vars in the original code.
    3. appendTimestamp() use the outputs from the other two functions to display desired result.

  

  var timeInterval = 1000;

function showCurrentTime(){
    var timestamp = getCurrentTimestamp();

    var timestampString = buildTimestampString(timestamp);
   
    appendTimestamp(timestampString, timestamp);   
};

function getCurrentTimestamp(){
  var months = new Array('January', 'Febuary', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December');
  var dateObj = new Date();
  
  var timestamp = {
    day : dateObj.getDate(),
    month : months[dateObj.getMonth()],
    year : dateObj.getFullYear(),
    hour : dateObj.getHours(),
    minutes : (dateObj.getMinutes()<=9?'0'+dateObj.getMinutes():dateObj.getMinutes())
  }

  return timestamp;
}

function buildTimestampString(timestamp){
     var string  = timestamp.month+
    ' '+timestamp.day+
    ', '+timestamp.year+
    '<br />'+timestamp.hour+
    ':'+timestamp.minutes+
    ' '+(timestamp.hour<=11?'am':'pm');

    return string;
}

function appendTimestamp(timestampString, timestamp){
   var timeDiv = document.getElementById('time');

    if(timeDiv !== null) {
        timeDiv.innerHTML = timestampString;
        timeDiv.setAttribute('datetime',timestamp.year+'-'+
            (timestamp.month+1<=9?'0'+(timestamp.month+1):timestamp.month+1)+'-'+timestamp.day+' '+timestamp.hour+':'+timestamp.minutes);

    };  
}

setInterval(showCurrentTime,timeInterval);
<time id="time"></time>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.