Jump to content

JavaScript Help


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 comment
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 comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

  • Create New...