patternjavascriptMinor
Small Calendar "Widget"
Viewed 0 times
widgetsmallcalendar
Problem
I have been working on a calendar widget for the past hour or so. I wonder if it could be compacted any more than it already is (ignoring whitespace).
`$(document).ready(function() {
var d = new Date();
var cd = d.getDate();
var cm = d.getMonth() + 1;
var cy = d.getFullYear();
var M = cm;
var Y = cy;
var days = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"];
var months = ["January", "Febuary", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"];
monthDays = function(m, y) {
var d = new Date(y, m, 0).getDate();
return d;
};
firstDay = function(m, y) {
var d = new Date(y, m - 1, 1).getDay();
d = (d === 0) ? 7 : d
return d;
};
currentDay = function(d, m, y) {
var d = new Date(y, m - 1, d).getDay();
d = (d === 0) ? 7 : d
return d;
};
function calendar(M, Y) {
M = M || cm;
Y = Y || cy;
//calendar dates
var date = cd + '/' + ('0' + M).substr(-2) + '/' + ('000' + Y).substr(-4);
var totalDays = monthDays(M, Y);
var lastMonth = monthDays(M - 1, Y);
var firstday = firstDay(M, Y);
var actualDay = currentDay(cd, M, Y);
//counters
var dateCount = 1;
var dayCount = 0;
//prev padding
for (var i = 1; i ');
$('div.widget div.body table tr:last').append('
' + oldDate + '');
dayCount++;
if (dayCount == 7) {
$('div.widget div.body table tr:last').after('');
dayCount = 0;
};
};
//calendar
for (var i = 1; i ');
$('div.widget div.body table tr:last').append('
' + i + '');
dayCount++;
if (dayCount == 7) {
$('div.widget div.body table tr:last').after('');
dayCount = 0;
};
};
//next padding
if (dayCount != 0) {
var nextMonth = 0;
for (var i = dayCount; i
' + nextMonth + '');
if (i == 7)
$('div.widget div.body table tr:last').after('');
};
};
$('div
`$(document).ready(function() {
var d = new Date();
var cd = d.getDate();
var cm = d.getMonth() + 1;
var cy = d.getFullYear();
var M = cm;
var Y = cy;
var days = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"];
var months = ["January", "Febuary", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"];
monthDays = function(m, y) {
var d = new Date(y, m, 0).getDate();
return d;
};
firstDay = function(m, y) {
var d = new Date(y, m - 1, 1).getDay();
d = (d === 0) ? 7 : d
return d;
};
currentDay = function(d, m, y) {
var d = new Date(y, m - 1, d).getDay();
d = (d === 0) ? 7 : d
return d;
};
function calendar(M, Y) {
M = M || cm;
Y = Y || cy;
//calendar dates
var date = cd + '/' + ('0' + M).substr(-2) + '/' + ('000' + Y).substr(-4);
var totalDays = monthDays(M, Y);
var lastMonth = monthDays(M - 1, Y);
var firstday = firstDay(M, Y);
var actualDay = currentDay(cd, M, Y);
//counters
var dateCount = 1;
var dayCount = 0;
//prev padding
for (var i = 1; i ');
$('div.widget div.body table tr:last').append('
' + oldDate + '');
dayCount++;
if (dayCount == 7) {
$('div.widget div.body table tr:last').after('');
dayCount = 0;
};
};
//calendar
for (var i = 1; i ');
$('div.widget div.body table tr:last').append('
' + i + '');
dayCount++;
if (dayCount == 7) {
$('div.widget div.body table tr:last').after('');
dayCount = 0;
};
};
//next padding
if (dayCount != 0) {
var nextMonth = 0;
for (var i = dayCount; i
' + nextMonth + '');
if (i == 7)
$('div.widget div.body table tr:last').after('');
};
};
$('div
Solution
What exactly do you mean with "compacted"? Compactness isn't really something you should go for software development, since usually it doesn't have any advantages (performance or otherwise). You should instead be focused on functionality and readability of the source code.
This leads directly to the first point of critic: The variable names. What are
You can always uses a minifier for production code, if you want to improve load times.
All three could have better names, e.g.
Also the functions
The parameters of the function
Don't repeat jQuery selectors. Run
Instead of searching for
The use of paragraphs in the table cells (also later in the HTML) seems unnecessary.
All three loops are virtually identical and I believe the use of the variable
In the first click handler you define
The use of HTML elements seems a bit random. Why is the day of the month a level 1 header and the other parts paragraphs?
Consider using
This leads directly to the first point of critic: The variable names. What are
cm vs M and cy vs Y? Use long(er), expressive variable names. Also variables shouldn't start with capital letters in JavaScript unless they are classes. You can always uses a minifier for production code, if you want to improve load times.
monthDays, firstDay and currentDay definitions are lacking the var keyword putting them into the global namespace.All three could have better names, e.g.
monthDays => daysInMonth, firstDay => weekdayOfFirstDay, currentDay => weekday. (Generally you should use the phrases "weekday" and "day of month" in function and variable names. "Day" and "date" alone are too ambiguous.)Also the functions
firstDay and currentDay are almost identical.The parameters of the function
calendar shadow the variables M and Y.('0' + M).substr(-2) should be extracted into a function, e.g.function padWithZeros(value, length) {
var zeros = Array(length).join('0'); // Repeats '0' length-1 times.
return (zeros + value).substr(-length);
}dateCount isn't used.Don't repeat jQuery selectors. Run
$('div.widget div.body table') once and store it in a variable.Instead of searching for
$('div.widget div.body table tr:last') each time, store a reference to the table row when you create it:// before loop
var calenderTable = $('div.widget div.body table');
var currentRow;
// inside loop
if (dayCount == 0)
currentRow = $('').appendTo(calenderTable);
currentRow.append('' + oldDate + '');$('div.widget div.body table tr:last').after(''); does absolutely nothing. ...append("") already appended the "complete" element. Keep in mind you are manipulating the DOM here, not generating HTML source code. As a reminder you should use ...append("") instead. The use of paragraphs in the table cells (also later in the HTML) seems unnecessary.
All three loops are virtually identical and I believe the use of the variable
currentDay is unnecessary. The weekday should be calculable from the loop index. (EDIT: added missing unnecessary.)In the first click handler you define
trg but don't use it.widget is a far too generic class name, and I don't understand what trg stands for.The use of HTML elements seems a bit random. Why is the day of the month a level 1 header and the other parts paragraphs?
Consider using
thead and th elements in the table. That way you can also get rid of the duplicate class name head.Code Snippets
function padWithZeros(value, length) {
var zeros = Array(length).join('0'); // Repeats '0' length-1 times.
return (zeros + value).substr(-length);
}// before loop
var calenderTable = $('div.widget div.body table');
var currentRow;
// inside loop
if (dayCount == 0)
currentRow = $('<tr>').appendTo(calenderTable);
currentRow.append('<td class="prev"><p>' + oldDate + '</p></td>');Context
StackExchange Code Review Q#101294, answer score: 3
Revisions (0)
No revisions yet.