patternjavascriptMinor
JavaScript clock
Viewed 0 times
javascriptclockstackoverflow
Problem
I am trying to build a clock. You can see the working code here. However, I feel like I can do much better in the JavaScript code.
var getDate = function getDate() {
var date = new Date();
var month = date.getMonth();
var day = date.getDate();
var hour = date.getHours();
var minutes = date.getMinutes();
var monthNames = ['January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December'];
var ampm = 'pm'; // by default, it is pm
var showDate;
var showTime;
if (hour 12) {
hour -= 12;
}
showDate = monthNames[month] + ' ' + day;
showTime = hour + ':' + minutes + ampm;
document.getElementById('js-date').innerHTML = showDate;
document.getElementById('js-time').innerHTML = showTime;
requestAnimationFrame(getDate);
};
getDate();Solution
A few improvements:
Added some line breaks to better group the functionalities.
Better variable name for
You could capitalize the time period.
Some people would prefer to collapse this
Added a space between time string and time period string.
You might want to rename your
You must not use requestAnimationFrame for this application. It is typically called 60 times in a second, and it would be wasteful to update your watch. You can just use a setInterval as shown below. This would trigger your getDate function every second to update the UI - which is more than enough for your purpose. I would go a step further and set the duration to 60 seconds, as your watch does not display the seconds.
var getDate = function getDate() {Added some line breaks to better group the functionalities.
var date = new Date();
var month = date.getMonth();
var day = date.getDate();
var hour = date.getHours();
var minutes = date.getMinutes();
var monthNames = ['January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December'];Better variable name for
ampm, and setting empty string as default value.var timePeriod = '';
var dateString;
var timeString;
if (hour < 10) {
hour = '0' + hour;
}
if (minutes < 10) {
minutes = '0' + minutes;
}You could capitalize the time period.
if (hour < 12) {
timePeriod = 'AM';
} else {
timePeriod = 'PM';
}Some people would prefer to collapse this
if block into one line using hour = hour % 12, but yours is okay given it is simpler to understand.if (hour > 12) {
hour -= 12;
}
dateString = monthNames[month] + ' ' + day;Added a space between time string and time period string.
timeString = hour + ':' + minutes + ' ' + timePeriod;You might want to rename your
span blocks to something like 'date' and 'time', instead of 'js-date' and 'js-time' because ID does not have to be an indicator of how the span gets its value.document.getElementById('js-date').innerHTML = dateString;
document.getElementById('js-time').innerHTML = timeString;You must not use requestAnimationFrame for this application. It is typically called 60 times in a second, and it would be wasteful to update your watch. You can just use a setInterval as shown below. This would trigger your getDate function every second to update the UI - which is more than enough for your purpose. I would go a step further and set the duration to 60 seconds, as your watch does not display the seconds.
// requestAnimationFrame(getDate);
};
setInterval(function() {
getDate();
}, 1000);Code Snippets
var getDate = function getDate() {var date = new Date();
var month = date.getMonth();
var day = date.getDate();
var hour = date.getHours();
var minutes = date.getMinutes();
var monthNames = ['January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December'];var timePeriod = '';
var dateString;
var timeString;
if (hour < 10) {
hour = '0' + hour;
}
if (minutes < 10) {
minutes = '0' + minutes;
}if (hour < 12) {
timePeriod = 'AM';
} else {
timePeriod = 'PM';
}if (hour > 12) {
hour -= 12;
}
dateString = monthNames[month] + ' ' + day;Context
StackExchange Code Review Q#155671, answer score: 3
Revisions (0)
No revisions yet.