[Code Review] JavaScript Calendar

Hello!

I created a JavaScript calendar that updates by itself to show the desired content.
It uses CSS as well for positioning.

Here is the main part of the code:


window.onload = update;

function update(){

var d = new Date();
var current = d.getDate();

var array = new Array(changeText, changeText2, changeText3, changeText4, changeText5, changeText6,changeText7, changeText8, changeText9, changeText10, changeText11, changeText12, changeText13, changeText14, changeText15, changeText16, changeText17, changeText18, changeText19, changeText20, changeText21, changeText22, changeText23, changeText24, changeText25, changeText26, changeText27, changeText28, changeText29, changeText30, changeText31);

var i = current - 1;

document.getElementById('content').innerHTML = array[i](); 
array[i]();

}

Here is an example of one of the functions called in the array:


function changeText(){
	document.getElementById('content').innerHTML = '<h1><img src="img/1.jpg" width="500px" height="500px" border="0" alt="1"/></h1><h2>September 1</h2><h3><iframe src="clock.html" frameborder="0" scrolling="no" marginheight="0" marginwidth="0" width="110px" height="40px" title="Clock"><noframes><div id="noframes"><span>Frames disabled.</span></div></noframes></iframe></h3>';
}



There is also a clock in the calendar that has its own code:


function startclock()
{
var thetime=new Date();

var nmonth=thetime.getMonth();
var nday=thetime.getDay();
var nhours=thetime.getHours();
var nmins=thetime.getMinutes();
var nsecn=thetime.getSeconds();
var AorP=" ";

if (nhours>=12)
    AorP="P.M.";
else
    AorP="A.M.";

if (nhours>=13)
    nhours-=12;

if (nhours==0)
 nhours=12;

if (nsecn<10)
 nsecn="0"+nsecn;

if (nmins<10)
 nmins="0"+nmins;

document.clockform.clockspot.value = nhours+":"+nmins+":"+nsecn+" "+AorP;

setTimeout('startclock()',1000);

} 

I wanted to know if anybody had any ideas or wanted to help make it a little more advanced.

Here is the link to the finished product:

http://www.cemidesign.com/calendar/

This is the source files:

http://www.cemidesign.com/calendar.zip

first consider your image swap code

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" lang="en">
<head>

<link rel="stylesheet" type="text/css" href="http://www.cemidesign.com/calendar/daily.css"/>
<script type="text/javascript" src="clock.js"></script>
<script type="text/javascript">
/*<![CDATA[*/

function update(){

 changeText(new Date().getDate())
 for (var z0=0,ary=[];z0<31;z0++){ // preload images
  ary[z0]=new Image();
  ary[z0].src='http://www.cemidesign.com/calendar/img/'+(z0+1)+'.jpg';
 }
}

function changeText(n){
 document.getElementById('img').src = 'http://www.cemidesign.com/calendar/img/'+n+'.jpg';
}

window.onload = update;

/*]]>*/
</script>
</head>
<body>

<div id="content">

  <h1><img id="img" src="img/1.jpg" width="500px" height="500px" border="0" alt="1"/></h1>
  <h2>September 1</h2>

  <!-- Clock -->
  <h3>
    <iframe src="clock.html" frameborder="0" scrolling="no" marginheight="0" marginwidth="0" width="180px" height="40px" title="Clock">
      <noframes><div id="noframes"><span>Frames disabled.</span></div></noframes>
    </iframe>
  </h3>
  <!-- Clock -->

</div>

<div id="month">

  <div id="extended">September 2013</div>

  <div id="days">
    <div id="sun">Sun</div>
    <div id="mon">Mon</div>
    <div id="tue">Tue</div>
    <div id="wed">Wed</div>
    <div id="thu">Thu</div>
    <div id="fri">Fri</div>
    <div id="sat">Sat</div>
  </div>

  <div id="days2">

    <div id="one"><input type='button' onclick='changeText(1)' value='1'/></div>
    <div id="two"><input type='button' onclick='changeText(2)' value='2'/></div>
    <div id="three"><input type='button' onclick='changeText(3)' value='3'/></div>
    <div id="four"><input type='button' onclick='changeText(4)' value='4'/></div>
    <div id="five"><input type='button' onclick='changeText(5)' value='5'/></div>

    <div id="six"><input type='button' onclick='changeText(6)' value='6'/></div>
    <div id="seven"><input type='button' onclick='changeText(7)' value='7'/></div>
    <div id="eight"><input type='button' onclick='changeText(8)' value='8'/></div>
    <div id="nine"><input type='button' onclick='changeText(9)' value='9'/></div>
    <div id="ten"><input type='button' onclick='changeText(10)' value='10'/></div>
    <div id="eleven"><input type='button' onclick='changeText(11)' value='11'/></div>
    <div id="twelve"><input type='button' onclick='changeText(12)' value='12'/></div>

    <div id="thirteen"><input type='button' onclick='changeText(13)' value='13'/></div>
    <div id="fourteen"><input type='button' onclick='changeText(14)' value='14'/></div>
    <div id="fifteen"><input type='button' onclick='changeText(15)' value='15'/></div>
    <div id="sixteen"><input type='button' onclick='changeText(16)' value='16'/></div>
    <div id="seventeen"><input type='button' onclick='changeText(17)' value='17'/></div>
    <div id="eighteen"><input type='button' onclick='changeText(18)' value='18'/></div>
    <div id="nineteen"><input type='button' onclick='changeText(19)' value='19'/></div>

    <div id="twenty"><input type='button' onclick='changeText(20)' value='20'/></div>
    <div id="twentyOne"><input type='button' onclick='changeText(21)' value='21'/></div>
    <div id="twentyTwo"><input type='button' onclick='changeText(22)' value='22'/></div>
    <div id="twentyThree"><input type='button' onclick='changeText(23)' value='23'/></div>
    <div id="twentyFour"><input type='button' onclick='changeText(24)' value='24'/></div>
    <div id="twentyFive"><input type='button' onclick='changeText(25)' value='25'/></div>
    <div id="twentySix"><input type='button' onclick='changeText(26)' value='26'/></div>

    <div id="twentySeven"><input type='button' onclick='changeText(27)' value='27'/></div>
    <div id="twentyEight"><input type='button' onclick='changeText(28)' value='28'/></div>
    <div id="twentyNine"><input type='button' onclick='changeText(29)' value='29'/></div>
    <div id="thirty"><input type='button' onclick='changeText(30)' value='30'/></div>

    <!--<div id="thirtyOne"><input type='button' onclick='changeText31()' value='31'/></div>-->

 </div>

</div>

</body>
</html>

we can look at your calender later if you want

I added the code 2 pre-load the images but I don’t know if it is working or not.

Hi,

Personally, I would look at refactoring what you have already, before adding new functionality.

In that respect, there are a couple of low hanging fruit, so let’s look at those first.

To start, let’s think about how to organize our code.
It would make sense to move all of the scripts to the bottom of the page, as this will eliminate the need to attach an “onload” event handler directly to the body tag.

This would give us this (I’ve also altered the doctype and added a <title> tag):

<!DOCTYPE HTML>
<html>
  <head>
    <meta charset="utf-8">
    <title>Calendar</title>
    <link rel="stylesheet" type="text/css" href="daily.css"/>
  </head>
  
  <body>
    <div id="content">
        
      <h1><img src="img/1.jpg" width="500px" height="500px" border="0" alt="1"/></h1>
      <h2>September 1</h2>
    
      <!-- Clock -->
      <h3>  
        <iframe src="clock.html" frameborder="0" scrolling="no" marginheight="0" marginwidth="0" width="180px" height="40px" title="Clock">
          <noframes><div id="noframes"><span>Frames disabled.</span></div></noframes>
        </iframe> 		 
      </h3>
      <!-- Clock -->
    
    </div>  
      
    <div id="month">
      <div id="extended">September 2013</div>
          
      <div id="days">
        <div id="sun">Sun</div>
        <div id="mon">Mon</div>
        <div id="tue">Tue</div>
        <div id="wed">Wed</div>
        <div id="thu">Thu</div>
        <div id="fri">Fri</div>
        <div id="sat">Sat</div>		
      </div>
          
      <div id="days2">
        <div id="one"><input type='button' onclick='changeText()' value='1'/></div>			
        <div id="two"><input type='button' onclick='changeText2()' value='2'/></div>			
        <div id="three"><input type='button' onclick='changeText3()' value='3'/></div>			
        <div id="four"><input type='button' onclick='changeText4()' value='4'/></div>			
        <div id="five"><input type='button' onclick='changeText5()' value='5'/></div>
          
        <div id="six"><input type='button' onclick='changeText6()' value='6'/></div>					
        <div id="seven"><input type='button' onclick='changeText7()' value='7'/></div>
        <div id="eight"><input type='button' onclick='changeText8()' value='8'/></div>			
        <div id="nine"><input type='button' onclick='changeText9()' value='9'/></div>			
        <div id="ten"><input type='button' onclick='changeText10()' value='10'/></div>
        <div id="eleven"><input type='button' onclick='changeText11()' value='11'/></div>			
        <div id="twelve"><input type='button' onclick='changeText12()' value='12'/></div>
          
        <div id="thirteen"><input type='button' onclick='changeText13()' value='13'/></div>						
        <div id="fourteen"><input type='button' onclick='changeText14()' value='14'/></div>
        <div id="fifteen"><input type='button' onclick='changeText15()' value='15'/></div>			
        <div id="sixteen"><input type='button' onclick='changeText16()' value='16'/></div>			
        <div id="seventeen"><input type='button' onclick='changeText17()' value='17'/></div>
        <div id="eighteen"><input type='button' onclick='changeText18()' value='18'/></div>					
        <div id="nineteen"><input type='button' onclick='changeText19()' value='19'/></div>
          
        <div id="twenty"><input type='button' onclick='changeText20()' value='20'/></div>						
        <div id="twentyOne"><input type='button' onclick='changeText21()' value='21'/></div>
        <div id="twentyTwo"><input type='button' onclick='changeText22()' value='22'/></div>				
        <div id="twentyThree"><input type='button' onclick='changeText23()' value='23'/></div>			
        <div id="twentyFour"><input type='button' onclick='changeText24()' value='24'/></div>
        <div id="twentyFive"><input type='button' onclick='changeText25()' value='25'/></div>		
        <div id="twentySix"><input type='button' onclick='changeText26()' value='26'/></div>
          
        <div id="twentySeven"><input type='button' onclick='changeText27()' value='27'/></div>						
        <div id="twentyEight"><input type='button' onclick='changeText28()' value='28'/></div>
        <div id="twentyNine"><input type='button' onclick='changeText29()' value='29'/></div>			
        <div id="thirty"><input type='button' onclick='changeText30()' value='30'/></div>		
        
        <!--<div id="thirtyOne"><input type='button' onclick='changeText31()' value='31'/></div>-->			
     </div>
    </div>

    <script type="text/javascript" src="daily.js"></script>
    <script type="text/javascript" src="clock.js"></script>
  </body>
</html>

Now the next thing that we could improve is to remove all of the onclick event handlers from the buttons and attach them from within the JavaScript itself.

This separation of a web page’s behaviour from its presentation is known as unobtrusive JavaScript.
It is considered a good practice and makes your code more flexible and easier to maintain.
You can read more about it here: http://en.wikipedia.org/wiki/Unobtrusive_JavaScript

var inputs = document.getElementsByTagName('input');
			
for(var i = 0; i < inputs.length; i++) {
  if(inputs[i].type.toLowerCase() == 'button') {
    inputs[i].onclick = function(){
      changeText(this.value);
    }
  }
}

This brings us nicely to the next improvement: instead of having thirty individually named functions that do the same thing, you can create one generic function, pass it a parameter and have it behave accordingly.

This would look like this:

// daily.js 
window.onload = update;

function update(){
	var d = new Date();
	var current = d.getDate();
	var array = new Array(changeText, changeText2, changeText3, changeText4, changeText5, changeText6,changeText7, changeText8, changeText9, changeText10, changeText11, changeText12, changeText13, changeText14, changeText15, changeText16, changeText17, changeText18, changeText19, changeText20, changeText21, changeText22, changeText23, changeText24, changeText25, changeText26, changeText27, changeText28, changeText29, changeText30, changeText31);
	var i = current - 1;
	document.getElementById('content').innerHTML = array[i](); 
	array[i]();
}

function changeText(val){
	document.getElementById('content').innerHTML = '<h1><img src="img/' + val + '.jpg" width="500px" height="500px" border="0" alt="' + val + '"/></h1><h2>September ' + val + '</h2><h3><iframe src="clock.html" frameborder="0" scrolling="no" marginheight="0" marginwidth="0" width="110px" height="40px" title="Clock"><noframes><div id="noframes"><span>Frames disabled.</span></div></noframes></iframe></h3>';
}

Now, as you can see we just removed 90 lines of JS but lost no functionality. Not bad, eh?

I hope this is ok for a start.
There are still lots and lots improvements that can be made and if you’re interested, I’ll be glad to help you make them.
Later on I’ll put the code up on JS fiddle.That way it’ll be easier to make incremental improvements.

Ok, fiddle can be found here.

I have removed the clock for the time being, so that we can concentrate on the calendar.
We can add this back later.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" lang="en">
<head>

<style type="text/css">
/*<![CDATA[*/
body {
margin:0;
top:0;
padding:0;
left:0;
max-width:1000px;
min-width:650px;
background-color:#111111;
}

#content{
position:absolute;
margin-top:5px;
margin-left:310px;
padding:10px;
width:600px;
height:600px;
background-color:#111111;
}

#content h1{
position:absolute;
margin-top:60px;
margin-left:5px;
width:500px;
height:500px;
border:1px solid #FFFFFF;
background-color:transparent;
}

#content h2{
position:absolute;
margin-top:0px;
margin-left:5px;
padding:5px;
font:36px Arial,"lucida console", sans-serif;
text-align:left;
font-weight:bold;
color:#FFFFFF;
background-color:#333333;
}

#content h3{
position:absolute;
margin-top:5px;
margin-left:403px;
width:180px;
height:38px;
}
#noframes{
position:absolute;
width:180px;
height:40px;
}

#noframes span{
margin-top:3px;
font:5px Arial,"lucida console", sans-serif;
text-align:center;
color:#303277;
}

#month{
margin-top:10px;
margin-left:10px;
width:270px;
height:280px;
border:1px solid #FFFFFF;
background-color:#111111;
}

#extended{
width:170px;
margin-top:15px;
margin-left:15px;
padding:4px;
border:1px solid #FFFFFF;
font:13px Arial,"lucida console", sans-serif;
text-align:left;
font-weight:bold;
color:#FFFFFF;
background-color:#222222;
}

#days{
margin-top:10px;
margin-left:5px;
width:250px;
height:20px;
background-color:transparent;
}

#days DIV {
float:left;
margin-left:5px;
width:29px;
font:12px Arial,"lucida console", sans-serif;
text-align:center;
color:#FFFFFF;
}

#days2{
margin-top:5px;
margin-left:5px;
width:250px;
height:20px;
background-color:transparent;
}

#days2 DIV {
float:left;
margin-top:5px;
margin-left:5px;
width:28px;
height:24px;
color:white;
text-Align:center;
padding-Top:2px;
background-color:transparent;
border:solid white 1px;
cursor:pointer;
}

#clock{
position:absolute;
left:500px;
top:20px;
width:223px;
height:23px;
font:20px Arial,"lucida console", sans-serif;
font-weight:bold;
text-align:center;
color:#FFFFFF;
border:1px solid #FFFFFF;
}

/*]]>*/
</style>

<script type="text/javascript">
/*<![CDATA[*/

function update(){
 var os=0;                                                    // date offset, 0 = mmddyyyy, 1 = ddmmyyyy
 var d,ary=[],z0=os,z1=0,z2=0;                                // general variables
 var months=['January','Febuary','March','April','May','June','July','August','September','October','November','December']; // month names
 var nms=['Sun','Mon','Tue','Wed','Thu','Fri','Sat','Sun'];                                                                 // day names
 var date=new Date();                                         // date variable
 var y=date.getFullYear(),m=date.getMonth();                  // year month variables

 changeText(date.getDate());
 startclock();
 document.getElementById('extended').innerHTML=months[m]+' '+y;
 document.getElementById('title').innerHTML=months[m]+' '+date.getDate();

// the day names
 for (;z0<7+os;z0++){
  d=document.createElement('DIV');
  d.innerHTML=nms[z0];
  document.getElementById('days').appendChild(d);
 }

// the days
 var fst=new Date(y,m,1,1).getDay();                          // find the day number of the first day of the month
 var lst=new Date(y,m+1,1,-1).getDate();                      // find the number of days in the month(the next month-1 hour)
 os==1?fst=(fst+6)%7:null;                                    // adjust the first day for ddmmyyyy format
 for (;z1<42;z1++){
  d=document.createElement('DIV');
  if (z1>=fst&&z1-fst<lst){
   d.innerHTML=z1-fst+1;
   d.onmouseup=function(){ changeText(this.innerHTML); }     // add event calls
  }
  else {
   d.style.visibility='hidden';
  }
  document.getElementById('days2').appendChild(d);
 }

// preload images
 for (;z2<31;z2++){
  ary[z2]=new Image();
  ary[z2].src='http://www.cemidesign.com/calendar/img/'+(z2+1)+'.jpg';
 }
}

function changeText(n){
 document.getElementById('img').src = 'http://www.cemidesign.com/calendar/img/'+n+'.jpg';
}

function startclock(){
 var thetime=new Date();

 var nhours=thetime.getHours();
 var nmins=thetime.getMinutes();
 var nsecn=thetime.getSeconds();
 var AorP="A.M.";

 if (nhours>=12)
    AorP="P.M.";

 if (nhours>=13)
    nhours-=12;

 if (nhours==0)
   nhours=12;

 if (nsecn<10)
  nsecn="0"+nsecn;

 if (nmins<10)
  nmins="0"+nmins;

 document.getElementById('clock').innerHTML=nhours+": "+nmins+": "+nsecn+" "+AorP+" ";

 setTimeout(function(){ startclock(); },1000);

}

window.onload = update;

/*]]>*/
</script>
</head>
<body>

<div id="content">

  <h1><img id="img" src="img/1.jpg" width="500px" height="500px" border="0" alt="1"/></h1>
  <h2 id="title" >September 1</h2>

  <!-- Clock -->
   <div id="clock" ></div>
  <!-- Clock -->

</div>

<div id="month">

  <div id="extended">September 2013</div>

  <div id="days">
  </div>

  <div id="days2">
  </div>

</div>


</body>
</html>

Thanks 4 the feedback guys.
I am looking @ the new code and it looks good.
I will post any further suggestions 4 the update.

:cool2:

Ok so i studied the code and understand the enhancements
provided by the helpful people here.

Here are the updated links and finished product:

http://www.cemidesign.com/calendar/

This is the source files:

http://www.cemidesign.com/calendar.zip

Also can anybody help me upload this to fiddle so other people can find it.

Here is my fiddle attempt: http://jsfiddle.net/vGQnm/

Hi again,

Are you sure you want to stop there?
There are a lot of improvements that can be made, and so far it is just a calendar for September.
Wouldn’t it be nicer if it was for the entire year?
Also, the readability of your code isn’t great, e.g.

 if (z1>=fst&&z1-fst<lst){
   d.innerHTML=z1-fst+1;
  ...
}

and you have a lot of duplication in your HTML.

One thing that jumps out is that you need to alter the src attribute of your image tags to point at wherever you have the images hosted.

<img id="img" src="http://myserver.com/img/1.jpg" width="500px" height="500px" border="0" alt="1"/>