Digital Clock Script

Question 1

While learning JavaScript I thought it was a good idea to make a digital clock program. I followed a youtube tutorial (with some tweaks to my preference) and came up with my final code that I have. I came to CodeReview to get the script reviewed, and to ask is there anything wrong with the script and/or any way to improve it as a whole. Ways to make the script shorter (yet also high in performance) are also accepted. I am also curious on if I have any 'red flags' currently. I will be giving the best answers the green checkmark. Thanks!

function showTime() {
 const date = new Date();
 let hours = date.getHours();
 let minutes = date.getMinutes();
 let seconds = date.getSeconds();
 let session = (hours < 12) ? "AM" : "PM";
 if (hours === 0) {
 hours = 12;
 } else if (hours > 12) {
 hours -= 12;
 }
 hours = (hours < 10) ? '0' + hours : hours;
 minutes = (minutes < 10) ? '0' + minutes : minutes;
 seconds = (seconds < 10) ? '0' + seconds : seconds;
 const time = `${hours} : ${minutes} : ${seconds} ${session}`;
 document.querySelector('div').innerHTML = time;
 setTimeout(showTime,1000)
}
showTime()

<!DOCTYPE html>
 <html>
 <head>
 <title>Digital Clock</title>
 </head>
 <body>
 <div></div>
 <script src = "script.js"></script>
 </body>
 </html>

TL;DR: I am a JavaScript programmer, and CSS currently isn't my suite, therefore I know that the clock can use some styling. How can the JavaScript code be improved?

Question 2

Some repeated code, i.e. (<X> < 10) ? '0' + <X> : <X>;, this can be factored into a utility in order to be more DRY. Using string::padStart you can ensure a string of minimum length is returned, padding the beginning of the string until it achieves the length.

The padStart() method pads the current string with another string (multiple times, if needed) until the resulting string reaches the given length. The padding is applied from the start of the current string.

When hours/minutes/seconds is small enough a value to become a single digit this pads it back to "two digits". We run it through the String constructor to make it a string and able to use string functions.

const padTime = value => String(value).padStart(2, '0');

You can also precompute the hours into the range [0, 11] by taking a modulus 12. The hours === 0 check will shift the hours to be in the range [1, 12].

Use a setInterval instead of setTimeout and move it outside the function body. You'll need to invoke the function once initially, but the interval will handle each subsequent invocation to update display. By moving out of the body you'll avoid clock skew, i.e. the time between 1000 timeout expiration and the next timeout instantiation.

Give the div you want to inject the time into a specific id so you can deterministically know where it'll be. I used an id of time.

function showTime() {
 const date = new Date();
 let hours = date.getHours();
 let minutes = date.getMinutes();
 let seconds = date.getSeconds();
 let session = (hours < 12) ? "AM" : "PM";
 // Modulus 12 "places" the hours into 1 of 12 "bins" [0-11]
 // Checking for 0 and setting to 12 shifts "bins" to [1-12]
 // hours = hours % 12 || 12; is more succinct using the fact
 // that `0` is a falsey value.
 hours = hours % 12; // hours one of [0,1,2,3,4,5,6,7,8,9,10,11]
 if (hours === 0) {
 hours = 12; // hours one of [1,2,3,4,5,6,7,8,9,10,11,12]
 }
 
 const padTime = value => String(value).padStart(2, '0');
 const time = `${padTime(hours)} : ${padTime(minutes)} : ${padTime(seconds)} ${session}`;
 document.querySelector('#time').innerHTML = time;
}
showTime();
setInterval(showTime, 1000);

<!DOCTYPE html>
 <html>
 <head>
 <title>Digital Clock</title>
 </head>
 <body>
 <div id="time"></div>
 <script src="script.js"></script>
 </body>
 </html>

Question 3

I liked your solution to get rid of the down vote. One question, can you explain more of what the pad means?

Question 4

@DanielP533 Thanks. Updated answer with a bit more explanation around padding the start of the string.

Question 5

1

You are repeating condition ? '0' + integer : integer 3 times.

This can be reduced with const zeroPad = (num, places) => String(num).padStart(places, '0')

and doing

 hours = zeroPad(hours, 2);
 minutes = zeroPad(minutes, 2);
 seconds = zeroPad(seconds, 2);

The 2 is now a repeated magic number and you may wish to store it in a variable with a descriptive name.

2

<script src = "script.js"></script>

Usually, there are no spaces around the equals sign:

<script src="script.js"></script>

3

I'm not familiar with JavaScript, but I would replace the nested setTimeout:

function showTime() {
 // ...
 setTimeout(showTime, 1000)
}

with a setInterval:

setInterval(showTime, 1000);

I would also place a space after the comma.

4

For formatting the time, take a look at this answer which suggests using:

hours = hours % 12;
hours = hours ? hours : 12;

5

You use document.querySelector('div') but this "returns the first Element within the document that matches the specified selector".

I would suggest using <div id="..."></div> and document.getElementById

Question 6

I assume, that you are exercising formatting and handling time correctly, so I won't mention, that you could do it in a "oneliner" using the existing api (for the options see here):

 let options = {
 hour12: true,
 hour: "2-digit",
 minute: "2-digit",
 second: "2-digit"
 };
 function showTime() {
 container.textContent = (new Date()).toLocaleTimeString("en-US", options);
 }
setInterval(showTime, 1000);

Another way to format a number by is again to use the existing api instead of String.padStart():

 let x = 42;
 console.log(x.toLocaleString("en-US", { minimumIntegerDigits: 3 }));

Here the option speaks for itself, and the output is "042".

The other answers address the most to be said about your code. One thing not mentioned is, that your watch potentially can be as much as nearly one second behind the actual time, if it is started somewhere in between two seconds. To align with the system time, you could do the following:

setTimeout(() => setInterval(showTime, 1000), 1000 - (new Date()).getMilliseconds());

Here setTimeout() is waiting for the next system's second to occur, and then starting the watch by calling setInterval().

Question 7

For now, you're the best answer

Question 8

Nevermind, your code has an error and does not work.

Question 9

@DanielP533: OK, so what is the error?

Question 10

Nevermind, fixed

Question 11

Does IE and "firefox for andrioid" support this method though? The first one? Mozilla docs says "IANA time zone names in timeZone option " then gives it an X in both of the browsers

Question 12

It's looking good and it works. A few remarks within the code

function showTime() {
 const date = new Date();
 let hours = date.getHours();
 let minutes = date.getMinutes();
 let seconds = date.getSeconds();
 let session = (hours < 12) ? "AM" : "PM";
 // You can stringify the minutes and seconds using a template string, 
 // for later zero padding (see snippet).
 if (hours === 0) {
 hours = 12;
 } else if (hours > 12) {
 hours -= 12;
 }
 // ^ this can be a bit shorter using a ternary operator
 // or you can even do without it using hours % 12 (see snippet)
 hours = (hours < 10) ? '0' + hours : hours;
 minutes = (minutes < 10) ? '0' + minutes : minutes;
 seconds = (seconds < 10) ? '0' + seconds : seconds;
 // ^ modern es20xx knows a native string method called 'padStart'
 const time = `${hours} : ${minutes} : ${seconds} ${session}`;
 document.querySelector('div').innerHTML = time;
 // ^ you may want to use textContent 
 // ^ if there's more in your document you may want to use an identifier 
 // (e.g. id or a data-attribute)
 setTimeout(showTime,1000)
 // ^ do not forget semicolons
}
showTime()
// ^ do not forget semicolons

A note for the html within the snippet: you do not need a complete html document in code review snippets, your html will be automatically wrapped in a document.

For the rationale to use semicolons, see this article.

Why textContent? From MDN

It is not uncommon to see innerHTML used to insert text into a web page. There is potential for this to become an attack vector on a site, creating a potential security risk.

function showTime() {
 const date = new Date();
 // converting hour/minutes/seconds to zero padded strings right away
 const hours = `${(date.getHours() % 12) || 12}`.padStart(2, 0);
 // ^ hours: if the value is 0, js considers it to be 'falsy'
 // that's why we can use this 'short-circuit evaluation`
 const minutes = `${date.getMinutes()}`.padStart(2, 0);
 const seconds = `${date.getSeconds()}`.padStart(2, 0);
 const session = date.getHours() < 12 ? "AM" : "PM";
 // ^ because hours now is a string (and < 12)
 const time = `${hours} : ${minutes} : ${seconds} ${session}`;
 
 // use an identifier (data-attribute), and use textContent
 document.querySelector('div[data-isclock]').textContent = time;
 setTimeout(showTime,1000);
}
showTime();

<!-- added data-isclock -->
<div data-isclock="1"></div>

The clock method may even be shorter:

const currentTime = date => 
 `${`${(date.getHours() % 12) || 12}`.padStart(2, 0)} : ${
 `${date.getMinutes()}`.padStart(2, 0)} : ${
 `${date.getSeconds()}`.padStart(2, 0)} ${
 date.getHours() < 12 ? "AM" : "PM"}`;
setInterval(() => 
 document.querySelector("div[data-isclock]")
 .textContent = currentTime(new Date()), 1000);

<div data-isclock></div>

Drew Reese Drew Reese 9391 gold badge6 silver badges14 bronze badges · Accepted Answer · 2020-06-12 06:31:55Z

Some repeated code, i.e. (<X> < 10) ? '0' + <X> : <X>;, this can be factored into a utility in order to be more DRY. Using string::padStart you can ensure a string of minimum length is returned, padding the beginning of the string until it achieves the length.

The padStart() method pads the current string with another string (multiple times, if needed) until the resulting string reaches the given length. The padding is applied from the start of the current string.

When hours/minutes/seconds is small enough a value to become a single digit this pads it back to "two digits". We run it through the String constructor to make it a string and able to use string functions.

const padTime = value => String(value).padStart(2, '0');

You can also precompute the hours into the range [0, 11] by taking a modulus 12. The hours === 0 check will shift the hours to be in the range [1, 12].

Use a setInterval instead of setTimeout and move it outside the function body. You'll need to invoke the function once initially, but the interval will handle each subsequent invocation to update display. By moving out of the body you'll avoid clock skew, i.e. the time between 1000 timeout expiration and the next timeout instantiation.

Give the div you want to inject the time into a specific id so you can deterministically know where it'll be. I used an id of time.

function showTime() {
 const date = new Date();
 let hours = date.getHours();
 let minutes = date.getMinutes();
 let seconds = date.getSeconds();
 let session = (hours < 12) ? "AM" : "PM";
 // Modulus 12 "places" the hours into 1 of 12 "bins" [0-11]
 // Checking for 0 and setting to 12 shifts "bins" to [1-12]
 // hours = hours % 12 || 12; is more succinct using the fact
 // that `0` is a falsey value.
 hours = hours % 12; // hours one of [0,1,2,3,4,5,6,7,8,9,10,11]
 if (hours === 0) {
 hours = 12; // hours one of [1,2,3,4,5,6,7,8,9,10,11,12]
 }
 
 const padTime = value => String(value).padStart(2, '0');
 const time = `${padTime(hours)} : ${padTime(minutes)} : ${padTime(seconds)} ${session}`;
 document.querySelector('#time').innerHTML = time;
}
showTime();
setInterval(showTime, 1000);

<!DOCTYPE html>
 <html>
 <head>
 <title>Digital Clock</title>
 </head>
 <body>
 <div id="time"></div>
 <script src="script.js"></script>
 </body>
 </html>

I liked your solution to get rid of the down vote. One question, can you explain more of what the pad means?
@DanielP533 Thanks. Updated answer with a bit more explanation around padding the start of the string.

Stack Exchange Network

Digital Clock Script

4 Answers 4

1

2

3

4

5

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Hot Network Questions

Digital Clock Script

4 Answers 4

1

2

3

4

5

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Related

Hot Network Questions