Jump to content

Recommended Posts

Would someone be able to comment this JavaScript clock for me please, thanks.

<script>function show2(){if (!document.all&&!document.getElementById)returnthelement=document.getElementById? document.getElementById("tick2"): document.all.tick2var Digital=new Date()var hours=Digital.getHours()var minutes=Digital.getMinutes()var seconds=Digital.getSeconds()var dn="PM"if (hours<12)dn="AM"if (hours>12)hours=hours-12if (hours==0)hours=12if (minutes<=9)minutes="0"+minutesif (seconds<=9)seconds="0"+secondsvar ctime=hours+":"+minutes+":"+seconds+" "+dnthelement.innerHTML="<b style='font-size:14;color:black;'>"+ctime+"</b>"setTimeout("show2()",1000)}window.onload=show2//--></script>

Link to post
Share on other sites

That's really, really old code that doesn't use best practices, so I updated it while adding comments.

<script>	function show2() { 		// get a reference to the element using its id attribute		theElement = document.getElementById("tick2");				// create a date object set to this exact moment		var Digital = new Date();				// get the hour		// note that it's on a 24-hour basis		var hours = Digital.getHours();				// get the minute		var minutes = Digital.getMinutes();				// get the second		var seconds = Digital.getSeconds();				// create a string with the initial value "PM"		var dn = "PM";				// if the hour is before noon, change the value of the string to "AM"		if (hours < 12) {			dn="AM";		}		// if the hour is greater then 12, subtract 12 so the hour is between 1-12		if (hours > 12) {			hours = hours - 12;		}		// if the hour is 0 (midnight), change it to 12		if (hours == 0) {			hours = 12;		}		// if the minute is between 0-9, put a leading 0 in front of it		if (minutes <= 9) {			minutes = "0" + minutes;		}		// if the second is between 0-9, put a leading 0 in front of it		if (seconds <= 9) {			seconds = "0" + seconds;		}		// using the data created above, build a digital time string		var ctime = hours + ":" + minutes + ":" + seconds + " " + dn;				// embed the time string in a <span> element with certain style attributes		theElement.innerHTML = "<span style='font-size:14;color:black;font-weight:bold;'>" + ctime + "</span>";				// set a timer that calls this function every 1000 milliseconds		setTimeout(show2, 1000);		}	// when the document finishes loading, call the show2 function	// this initiates the clock	window.onload = show2;</script>

What I changed: there is no longer a need to test for document.all (obsolete versions of Explorer) or document.getElementById. The last one has been a standard since the last century. :)I added spaces for readability. The internet is faster than it used to be, so there is no need to save a trivial number of characters.I added a semicolon at the end of each statement. This is not a rule, but a good habit.I changed the name of thelement to theElement, which is a lot more sensible. You'll be less inclined to mistype it later.It's bad practice to create a <b> element and add further style attributes to it. I changed it to a span and added a font-weight property to the attribute string. Ideally, all that would go in a style sheet. Even better practice would be to use setInterval instead of repeatedly calling setTimeout, but I'll leave that for you to figure out. Hope that helps.

Edited by Deirdre's Dad
  • Like 1
Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

  • Create New...