HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Calendar code review

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codereviewcalendar

Problem

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("");
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("" + mN[m] + " - " + y + "");
$("#calendar").append("");
for (var i = 0; i " + w[i][0] + "");
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 ").appendTo("#calendar");
}
if (day > 0) {
date = datDate(m, day, y);
if (day === d) {
$row.append("" + day + "");
} else {
$row.append("" + day + "");
}
} else {
var day = daysInLastMonth--;

Solution

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 = $('').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("");
for (var i = 0; i " + w[i][0] + "");


Instead do:

var day = jQuery("").appendTo(calender);
for (var i = 0; i " + w[i][0] + "");


(Maybe more later).

Code Snippets

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('...');
   // ...
}
$("#calendar").append("<tr class='day'></tr>");
for (var i = 0; i < 7; i++) $(".day").append("<td>" + w[i][0] + "</td>");
var day = jQuery("<tr></tr>").appendTo(calender);
for (var i = 0; i < 7; i++) day.append("<td>" + w[i][0] + "</td>");

Context

StackExchange Code Review Q#20691, answer score: 3

Revisions (0)

No revisions yet.