Calendar code review

Question

I wrote the following in Javascript and jQuery, and I'm wondering if there are any ways to improve the code, organize it better, and increase speed:

function calendar(d) {
    $("#calendar").remove();
    $("#calendar_container").append("<table border='1' id='calendar'></table>");
    t = new Date(d); // Today [Wed Jan 16 2013 00:00:00 GMT-0500 (EST)]
    var d = t.getDate(); // Today's date (1-31) [16]
    var y = t.getFullYear(); // Full year [2013]
    var m = t.getMonth(); // Month (0-11) [0]
    var mN = ["January", // Month name array (0-11)
    "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"];
    var fM = mN[m]; // Full month name [January]
    var dIM = new Date(y, m + 1, 0).getDate(); // Number of days in current month (1-31) [31]
    var dILM = new Date(y, m, 0).getDate(); // Number of days in last month (1-31) [31]
    var dOW = new Date(y, m, 1).getDay(); // Day of the first day of the month (0-6) [2]
    var nOW = Math.ceil((dIM + dOW) / 7); // Number of weeks in the month, including space [5]
    var w = ["Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"];

    function datDate(m, day, y) {
        date = new Date(" " + mN[m] + " " + day + " " + y);
        return date;
    }
    $("#calendar").append("<tr><td class='month' colspan='7'>" + mN[m] + " - " + y + "</td></tr>");
    $("#calendar").append("<tr class='day'></tr>");
    for (var i = 0; i < 7; i++) $(".day").append("<td>" + w[i][0] + "</td>");
    if (m === 0) {
        n_m = 12;
    } else {
        n_m = m - 1;
    }
    var pre = new Date(y - 1, n_m + 1, 0).getDate();
    var daysInLastMonth = new Date(y, n_m + 1, 0).getDate();
    var after = (7 * (nOW)) - (dOW + dIM);
    var count = 0,
        $row;
    for (var i = 0 - dOW; i < dIM; i++) {
        var day = i + 1;
        if (count % 7 === 0) {
            $row = $("<tr class='row'>").appendTo("#calendar");
        }
        if (day > 0) {
            date = datDate(m, day, y);
            if (day === d) {
                $row.append("<td class='calendar_date current_date watch' data-date='" + date + "'>" + day + "</td>");
            } else {
                $row.append("<td class='calendar_date watch' data-date='" + date + "'>" + day + "</td>");
            }
        } else {
            var day = daysInLastMonth--;
            if (m === 0) {
                var n_y = y - 1;
                var n_m = 11
            } else {
                var n_m = m - 1;
                var n_y = y;
            }
            var n_date = new Date(n_y, n_m, day);
            $row.prepend("<td class='calendar_back watch' data-date='" + n_date + "'>" + day + "</td>");
        }
        count++;
    }
    for (var i = 0; i < after; i++) {
        var day = (i + 1);
        if (m === 12) {
            var n_y = y + 1;
            var n_m = 0
        } else {
            var n_m = m + 1;
            var n_y = y;
        }
        var n_date = new Date(n_y, n_m, day);
        $("td:last").after("<td class='calendar_next watch' data-date='" + n_date + "'>" + day + "</td>");
    }
}

function calendarSetup() {
    function changed(c) {
        $(".current").text(c);
        localStorage.setItem("date", c);
        b_current = new Date(c);
        b_year = b_current.getFullYear();
        b_month = b_current.getMonth();
        b_date = b_current.getDate();
        b_daysInLastMonth = new Date(b_year, b_month, 1).getDate();
    }
    if (localStorage.getItem("date")) {
        var now = localStorage.getItem("date");
    } else {
        var now = new Date(); // "1 March 2013"
    }
    calendar(now);
    changed(localStorage.getItem("date"));
    b_current = new Date(now);
    b_year = b_current.getFullYear();
    b_month = b_current.getMonth();
    b_date = b_current.getDate();
    b_daysInLastMonth = new Date(b_year, b_month, 1).getDate();
    b_monthNames = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"];

    function move(a, b, c) {
        if (b_month === a) {
            var year = b_year + b;
            var month = c;
            a === 11 ? day = 1 : day = new Date(year, month + 1, 0).getDate();
        } else {
            var year = b_year;
            var month = b_month + b;
            a === 11 ? day = 1 : day = new Date(year, month + 1, 0).getDate();
        }
        var now = new Date(year, month, day);
        calendar(now);
        changed(now);
    }

    function step(a, b, c) {
        var year = b_year;
        var day = b_date + b;
        var month = b_month;
        var now = new Date(year, month, day);
        calendar(now);
        changed(now);
    }
    $(".watch").live("click", function () {
        if ($(this).hasClass("back")) {
            move(0, -1, 11);
        }
        if ($(this).hasClass("next")) {
            move(11, 1, 0);
        }
        if ($(this).hasClass("calendar_back")) {
            var now = $(this).data("date");
            calendar(now);
            changed(now);
        }
        if ($(this).hasClass("calendar_next")) {
            var b_now = $(this).data("date");
            calendar(b_now);
            changed(b_now);
        }
        if ($(this).hasClass("calendar_date")) {
            var b_now = $(this).data("date");
            calendar(b_now);
            changed(b_now);
        }
        if ($(this).hasClass("today")) {
            var b_now = new Date();
            calendar(b_now);
            changed(b_now);
        }
    });
    $(document).keydown(function (e) {
        if (e.keyCode == 37) {
            step(0, -1, 11);
        }
        if (e.keyCode == 39) {
            step(11, 1, 1);
        }
    });
}
// Onload
calendarSetup();

Demo: http://jsfiddle.net/charlescarver/XZE3x/

Answer 1

You shouldn't hard-code any IDs or use global variables, because either could collide with other HTML, JavaScript, etc. on the page.

Instead of hard-coding #calendar_container, pass the id of the target element as a parameter, and instead of giving your table the hard-coded id calendar, you could get a reference from jQuery when creating it:

function calendar(container, date) {
   var container = $(container);
   container.empty(); // Instead of removeing the table "#calender"
   var calender = $('<table></table>').appendTo(container); // Drop 'border=1'. Style the table in CSS.
   // From here on you can use the variable calendar intstead of $('#calendar') to refer to your table:
   // ...
   calendar.append('...');
   // ...
}

Similar here:

$("#calendar").append("<tr class='day'></tr>");
for (var i = 0; i < 7; i++) $(".day").append("<td>" + w[i][0] + "</td>");

Instead do:

var day = jQuery("<tr></tr>").appendTo(calender);
for (var i = 0; i < 7; i++) day.append("<td>" + w[i][0] + "</td>");

(Maybe more later).

Answer 2

The code you posted is quite big so I believe that's why no one has answered this question yet. For a better and more specific answer I would suggest you ask several questions with isolated sections you want to improve.

Anyways, here are some basic improvements you could use:

• Removing $("#calendar").remove(); that's not in your HTML? Then you call this later?
• You only need to say "var" once. After that just use a "," and bumb onto the next one.
• Use variable names that actually tell us what they are. Keeping this in mind will help you in the future if you have to update or make changes to your code. If you're worried about size, don't. You should be using a minifier anyways and that would replace your variable names later.

Ex:

 var d = t.getDate(), // Use the coma instead of saying var each time.
    y = t.getFullYear(), // Name your var "year" instead of just "y".
    m = t.getMonth(),
    mN = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"],
    fM = mN[m],
    dIM = new Date(y, m + 1, 0).getDate(),
    dILM = new Date(y, m, 0).getDate(),
    dOW = new Date(y, m, 1).getDay(),
    nOW = Math.ceil((dIM + dOW) / 7),
    w = ["Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"];

• As a rule of thumb, if you use a selection more than once you should cache its value:

Ex:

var calendar = $("#calendar");

calendar.append("<tr><td class='month' colspan='7'>" + mN[m] + " - " + y + "</td></tr>");
calendar.append("<tr class='day'></tr>");

• Why do you declare the month names twice? Use the previously declared array instead.

This:

mN = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"]
b_monthNames = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"];
//Just use the mN again instead of making a whole new variable.

• Use a IIFE instead of this calendarSetup();

Ex:

(function($) {
//Wrap your code with this.
//This gets immediatly invoked and runs your code.
//We pass "$" to the function as jQuery.
//By doing this you protect yourself from other libraries that might use the "$", thus, preventing conflicting code.
})(jQuery);

• Lastly, you should consider using a design pattern. You can do some research to find which might fit your situation the best. This book describes them well and provides examples etc. From what I can tell from your code I would recomend the Module Design Pattern. Chris Coyer does a good job explaining this concept in this article. I also highly recommend this course by Jeffery Way. He explains a lot of the core concepts in jQuery as well as some more advanced techniques.

Answer 3

You are storing loads of information in the class names and as data in the DOM which is ultimately making everything a lot more complicated. A better approach would be to store it in an object. But also note that there is only one small piece of information you need to know to display the calendar correctly: the date it is supposed to be showing. Most of the other variables you create are superfluous.

Splitting the logic between calendar and calendarSetup is particularly problematic because you end up recalculating and storing some of the same data in both (like the names of the months), and unnecessarily rebuilding the entire calendar HTML each time you change the date.

You could also make better use of javascript's date methods, for instance:

date.setMonth(date.getMonth() - 1)

subtracts a month from date (and works even if the month is January).

Here is a version that constructs an object for the calendar. (I used plain js, but it should be easy enough to add jquery.) (jsfiddle)

function nel(type, parent) {
    // helper to make or clone a new element and append it to parent
    var el = document.createElement(type);
    if (parent) parent.appendChild(el);
    return el;
}

function Calendar(elements) {
    var startDate, date = new Date(),
        table = nel('table', elements.container),
        headerCell = nel('th', nel('tr', table)),
        months = ['January', 'February', 'March', 'April', 'May', 'June',
            'July', 'August', 'September', 'October', 'November', 'December'],
        days = ['S', 'M', 'T', 'W', 'T', 'F', 'S'];

    function buildHtml() {
        var i, j, row, cell;
        headerCell.colSpan = 7;

        // Add days of week in first row
        row = nel('tr', table);
        for (i = 0; i < 7; i++) {
            nel('td', row).innerHTML = days[i];
        }

        // Add the remaining cells and give them event handlers      
        for (j = 0; j < 5; j++) {
            row = nel('tr', table);
            for (i = 0; i < 7; i++) {
                nel('td', row).onclick = getCellResponse(i, j);
            }
        }
    }

    function getCellResponse(col, row) {
        return function () {
            date = new Date(startDate);
            date.setDate(date.getDate() + row * 7 + col);
            refresh();
        };
    }

    function refresh() {
        var cell, row, col, d;
        startDate = new Date(date);
        startDate.setDate(1);                      // set to first of month
        startDate.setDate(1 - startDate.getDay()); // set to preceding Sunday
        for (row = 2, d = new Date(startDate); row < 7; row++) {
            for (col = 0; col < 7; col++) {
                cell = table.children[row].children[col];
                cell.innerHTML = d.getDate();
                cell.className = '';
                if (d.getMonth() !== date.getMonth()) {
                    cell.className = 'feint';
                } else if (d.getDate() === date.getDate()) {
                    cell.className = 'selected';
                } 
                d.setDate(d.getDate() + 1);
            }
        }
        headerCell.innerHTML = months[date.getMonth()] + '-' + date.getFullYear();
    }
    buildHtml();
    refresh();
    elements.today.onclick = function () {
        date = new Date();
        refresh();
    };
    elements.forward.onclick = function () {
        date.setMonth(date.getMonth() + 1);
        refresh();
    };
    elements.back.onclick = function () {
        date.setMonth(date.getMonth() - 1);
        refresh();
    };
}

var calendar = new Calendar({
    current: document.getElementsByClassName('current')[0],
    container: document.getElementById('calendarContainer'),
    today: document.getElementsByClassName('today watch')[0],
    back: document.getElementsByClassName('back watch')[0],
    forward: document.getElementsByClassName('next watch')[0]
});

Answer 4

Some time ago i wrote a multilanguage calendar for modern browsers.

this function returns an array of the months and days in narrow,short & long

var months,weekdays;
function setlng(l){
 var wd={narrow:[],short:[],long:[]},b=7,d=new Date(),l=l||navigator.language;
 while(d.getDay()>0){d.setDate(d.getDate()+1);}
 while(b--){
  d.setDate(d.getDate()-1);
  wd.narrow[b]=d.toLocaleDateString(l,{weekday:'narrow'});
  wd.short[b]=d.toLocaleDateString(l,{weekday:'short'});
  wd.long[b]=d.toLocaleDateString(l,{weekday:'long'});
 } 
 var m={narrow:[],short:[],long:[]},b=12,d=new Date(),l=l||navigator.language;
 while(d.getMonth()>0){d.setMonth(d.getMonth()+1)};
 while(b--){
  d.setMonth(d.getMonth()-1);
  m.narrow[b]=d.toLocaleDateString(l,{month:'narrow'});
  m.short[b]=d.toLocaleDateString(l,{month:'short'});
  m.long[b]=d.toLocaleDateString(l,{month:'long'});
 }
 months=m;weekdays=wd;
}
setlng();//en,ru,it ... whatever language

setlng() returns the current browser language else you can manually specify a language

like en,it,ru... so setlng('ru') creates an array of russian month and day names.

then (i don't remember why) used the protorype to create some functions to get these monthname,the days in the current month and the calendar itself.

Date.prototype.getMonthName=function(){return months.long[this.getMonth()];};
Date.prototype.daysInMonth=function(){
 return new Date(this.getFullYear(),this.getMonth()+1,0).getDate();
};

Date.prototype.calendar=function(){
 var numDays=this.daysInMonth();
 var startDay=new Date(this.getFullYear(),this.getMonth(),1).getDay();
 var buildStr ='<table>';
 buildStr+='<tr><td colspan=7>'+this.getMonthName()+' '+this.getFullYear()+'</td></tr>';
 buildStr+='<tr><td>'+weekdays.short.join('</td><td>')+'</td></tr>';
 buildStr+='<tr>';
 buildStr+=new Array(startDay+1).join('<td>&nbsp;</td>');
 var border=startDay;
 for(i=1;i<=numDays;i++){
  buildStr+='<td>'+i+'</td>';
  border++;
  if(((border%7)==0)&&(i<numDays)){
   buildStr+='</tr><tr>';
  }
 }
 while((border++%7)!=0) {
  buildStr+='<td>&nbsp;</td>';
 }
 buildStr+='</tr>';
 buildStr+='</table>';
 return buildStr;
}

as you can see the calendar function is really short and also relatively fast.

all you have to set the language & pass a date to the calendar function

document.body.innerHTML=(setlng('it'),new Date()).calendar();//italian calendar

or

element.appendChild(document.createElement('div')).innerHTML=(new Date()).calendar()

now to create a navigation (for example to jump to the next month) you can do something like this.

element.innerHTML=(new Date(now.getFullYear(), now.getMonth() + 1, 1)).calendar();

and so on...

style with css by adding a class to the table inside the function

and i leaved the arguments free so you can add custom ones for different types of calendars.

ps.: i used to return a simple string vs dom manipulation because the code is much shorter.and the code it self is just a test i made some time ago .. and it needs alot of modifications to ptoperly work.i posted only cause maybe you find something useful inside this functions.

Example

http://jsfiddle.net/TgdT7/1/