I'd like to know how I can refactor my JS code for usability. Currently my JavaScript uses some HTML to build out a simple calendar with the following functionality:
a. opens into the current month with today's date highlighted
b. allows you to view next and previous month
My goal was to build out a calendar with the previously mentioned functionality while also having the ability to accept an array of event objects to populate the calendar. Currently, my calendar does NOT accept an array of event objects but that feature will be for another time.
My goal with your help is to refactor the calendar to be less repetitive and more like functional programming.
You can view my Github with the HTML and CSS.
var createMonth = {
createMonthYearLabel: function () {
var me = this;
// create the label that appears on top of the calendar
var currentMonth = me.currentMonth; // the index of the current month ex. 0
var currentMonthName = me.months[ currentMonth ]; // get the name of current month ex. January
var monthYear = me.currentMonthYear = currentMonthName + " " + currentYear; // create ex. January 2016
me.$monthYearSpan.innerHTML = monthYear;
},
createDaysInMonth: function () {
var me = this;
var currentYear = me.currentYear,
currentMonth = me.currentMonth,
currentDateFull = new Date(currentYear, currentMonth);
var totalDaysInMonth = new Date(currentYear, currentMonth + 1, 0).getDate(); // all days in a month
var firstDay = new Date(currentYear, currentMonth, 1);
firstDay = /[A-Za-z]{3}/.exec( firstDay )[0]; // day of the week when the month first begins ex. Tue
var totalNumOfRows = 6; // using 6 rows to show an entire calendar month
var dayCounter = 0;
var daysOfWeek = [ 'Sun', 'Mon', 'Tue', 'Wed','Thu','Fri', 'Sat' ];
var setFirstDayOfMonth; // to be set once to true
for (var i = 0; i < totalNumOfRows; i++) {
var $row = me.$monthTable.insertRow( i ); // creating a row
daysOfWeek.forEach( function( day, index ){ // iterating through the days of the week
var $cell = $row.insertCell( index ); // adding a cell to a row
if ( day === firstDay && !setFirstDayOfMonth ) { // if first day of month not set & day equals first day
// happens once for setting first day of month
dayCounter++;
setFirstDayOfMonth = true;
$cell.innerHTML = dayCounter;
if ( me.currentMonthYear === me.todayMonthYear ) {
if ( dayCounter === me.todayDate ) {
$cell.className = 'today'; // in case the first day of the month is today
}
}
return;
}
if ( dayCounter === 0 || dayCounter === totalDaysInMonth ) {
// creating empty squares with no dates / placeholders
$cell.innerHTML = "";
$cell.className = 'nil';
return; // dayCounter will not be triggered on empty days
}
if ( dayCounter > 0 ) {
dayCounter++;
$cell.innerHTML = dayCounter;
if ( me.currentMonthYear === me.todayMonthYear ) {
if ( dayCounter === me.todayDate ) {
$cell.className = 'today'; // in case the first day of the month is today
}
}
}
});
};
}
};
function createCalendar () {
var me = this;
me.prevBtn = document.getElementsByClassName('left button')[0]; // left arrow on calendar
me.nextBtn = document.getElementsByClassName('right button')[0]; // right arrow on calendar
me.$monthYearSpan = document.getElementsByClassName('month-year')[0]; // month-year title
me.$monthTable = document.getElementsByClassName('current')[0]; // table
me.months = [ 'January', 'Feburary', 'March', 'April', 'May','June','July','August','September','October','November','December' ];
me.todayDateFull = new Date();
me.currentYear = me.todayDateFull.getFullYear();
me.currentMonth = me.todayDateFull.getMonth();
me.todayDate = me.todayDateFull.getDate();
me.currentMonthName = me.months[ me.currentMonth ]; // get the name of current month ex. January
me.todayMonthYear = me.currentMonthName + " " + me.currentYear;
var createMonthYearLabel = createMonth.createMonthYearLabel,
createDaysInMonth = createMonth.createDaysInMonth;
createMonthYearLabel.call( me );
createDaysInMonth.call( me );
prevBtn.onclick = function () { // goes back in time
var me = this;
me.$monthYearSpan.innerHTML = "";
me.$monthTable.innerHTML = "";
if ( me.currentMonth === 0 ) {
// decrease the year
me.currentMonth = 11;
me.currentYear--;
} else {
me.currentMonth--;
}
createMonthYearLabel.call( me );
createDaysInMonth.call( me );
}.bind( me );
nextBtn.onclick = function () { // goes forward in time
var me = this;
me.$monthYearSpan.innerHTML = "";
me.$monthTable.innerHTML = "";
if ( me.currentMonth === 11 ) {
// increase the year
me.currentMonth = 0;
me.currentYear++;
} else {
me.currentMonth++;
}
createMonthYearLabel.call( me );
createDaysInMonth.call( me );
}.bind( me );
}
createCalendar();
body {
background: #e0e0e0e;
}
#cal {
-webkit-box-shadow: 0px 3px 3px rgba(0, 0, 0, 0.25);
margin: 50px auto;
font: 13px/1.5 "Helvetica Neue", Helvatica, Arial, san-serif;
display:table; /* centers the entire calendar */
}
#cal .header {
cursor: default;
background: #cd310d;
background: -webit-gradient(linear, left top, left bottom, from(#b32b0c) to(#cd310d));
height: 55px;
position: relative;
color:#fff;
-webkit-border-top-left-radius: 5px;
-webkit-border-top-right-radius: 5px;
border-top-left-radius: 5px;
border-top-right-radius: 5px;
font-weight: bold;
text-shadow: 0px -1px 0 #87260C;
text-transform: uppercase;
}
#cal .header span {
display: inline-block;
line-height: 55px; /* centers item horizontal */
}
#cal .header .hook {
width: 9px;
height: 28px;
position: absolute;
bottom: 60%;
border-radius: 10px;
-webkit-border-radius: 10px;
background: #ececec;
background: -moz-linear-gradient(right top, #fff, #827e7d);
background: -webkit-gradient(linear, right top, right bottom, from(#fff), to(#827e7d));
box-shadow:0px -1px 2px rgba(0, 0, 0, 0.65 );
-moz-box-shadow:0px -1px 2px rgba(0, 0, 0, 0.65 );
-webkit-box-shadow:0px -1px 2px rgba(0, 0, 0, 0.65 );
}
.right.hook {
right: 15%;
}
.left.hook {
left: 15%;
}
#cal .header .button {
width: 24px;
text-align: center;
position: absolute;
}
#cal .header .left.button {
left: 0;
-webkit-border-top-left-radius: 5px;
border-top-left-radius: 5px;
border-right: 1px solid #ae2a0c;
}
#cal .header .right.button {
right: 0;
top: 0;
border-left: 1px solid #ae2a0c;
-webkit-border-top-right-radius: 5px;
-moz-border-radius-topright: 5px;
border-top-right-radius: 5px;
}
#cal .header .button:hover {
background: -moz-linear-gradient(top, #d94215, #bb330f);
background: -webkit-gradient(linear, left top, left bottom, from(#d94215), to(#bb330f));
}
#cal .header .month-year {
letter-spacing: 1px;
width: 100%;
text-align: center;
}
#cal table {
background: #fff;
border-collapse: collapse;
width: 100%;
}
#cal td {
color: #2b2b2b;
width: 80px;
height: 65px;
line-height: 30px;
text-align: center;
border: 1px solid #e6e6e6;
cursor: default;
}
#cal #days td {
height: 26px;
line-height: 26px;
text-transform: uppercase;
font-size: 90%;
color: #9e9e9e;
}
#cal #days td:not(:last-child) {
border-right: 1px solid #fff;
}
#cal #cal-frame td.today {
background: #ededed;
color: black;
border: 0;
}
#cal #cal-frame td:not(.nil):hover {
color: #fff;
text-shadow: #6C1A07 0px -1px;
background: #cd310d;
background: -webkit-linear-gradient(top, #b32b0c, #cd310d);
background: -moz-linear-gradient(linear, left top, left bottom, from(b32b0c), to(#cd310d));
-moz-box-shadow: 0px 0px 0px;
-webkit-box-shadow: 0px 0px 0px;
}
#cal #cal-frame td span {
font-size: 80%;
position: relative;
}
#cal #cal-frame td span:first-child {
bottom: 5px;
}
#cal #cal-frame td span:last-child {
top: 5px;
}
#cal #cal-frame .curr {
float: left;
}
#cal #cal-frame table.temp {
position: absolute;
}
<!DOCTYPE html>
<head>
<meta charset="utf-8">
<title>Calendar Widget</title>
</head>
<body>
<div id="cal">
<div class="header">
<span class="left button" id="prev">⟨</span>
<span class="left hook"></span>
<span class="month-year" id="label"></span>
<span class="right hook"></span>
<span class="right button" id="prev">⟩</span>
</div>
<table id="days">
<tr>
<td>Sunday</td>
<td>Monday</td>
<td>Tuesday</td>
<td>Wednesday</td>
<td>Thursday</td>
<td>Friday</td>
<td>Saturday</td>
</tr>
</table>
<div id="cal-frame">
<table class="current"></table>
</div>
</div>
</body>
</html>
1 Answer 1
You're final product is really, really good (nice work!), but your code could still use a few improvements. The biggest problems with your code is that it is quite tightly coupled and could use some improvements with its cohesiveness. These issues can make your code harder to read and develop.
The general idea of cohesion is to organize your code so that each section of it does one thing. Break out parts of your code that do similar things into their own function, module, or service (you'll be seeing a lot of this kind of stuff in my answer).
Make it a module
Try wrapping your code to into a module. This will prevent your code from adding functions to the global namespace which can prevent collision of names with other libraries.
(function calendarModule() {
var createMonth = {
// ...
};
function createCalendar() {
// ...
};
createCalendar();
})();
Here is a great link for more info on the use of modules.
Readability could be improved by making your code more cohesive
To be honest, I had quite a hard time following what your code was doing at times. My biggest struggle was dealing with they way you used this
and your me
variable.
I think you could improve your readability and also improve your codes cohesiveness by eliminating a lot of uses of the this
variable. Your throwing a lot of data around. It would be better to limit some function's data access to only what it truly needs. Here are a few examples:
First, The createMonthYearLabel
only needs access to the month, the year, and the span it writes to, so why not pass them as parameters within their own object?
function createMonthYearLabel(data) {
data.$monthYearSpan.innerHTML = data.month + " " + data.year;
}
data.createMonthYearLabel({
$monthYearSpan : // ...,
month : // ...,
year : // ...
});
This clarifies what your function specifically does and does not need.
*Here's a stack overflow discussion on using a single parameter object.
Next, consider making a service object for accessing the elements on the DOM. Anyway of breaking out this code would be fine but here's how I would do it.
// I use this function to load data only once
// You can use it to query the DOM elements only once
function createCachingLoadFunction(load) {
var cache;
return function () {
return cache !== undefined ? cache : cache = load();
}
}
var elementsService = {
getLeftButton: createCachingLoadFunction(function () {
return document.getElementsByClassName('left button')[0];
},
// ...
}
Create a model object for your date data, including the functions that manipulate it:
calendarModel = (function () {
var date = new Date();
var year = date.getFullYear();
var month = date.getMonth();
// ...
return {
getYear: function () {
return year;
},
getMonth: function () {
return month;
},
// ... rest of getters
decrementMonth: function () {
// logic for "going back in time"
},
incrementMonth: function () {
// logic for "going forward in time"
}
};
})();
prevButton.onclick = calendarModel.decrementMonth;
nextButton.onclick = calendarModel.incrementMonth;
Possible Extension while further improving cohesiveness
You could extract all your constant data like daysOfTheWeek
and months
into a service object.
(function calendarModule() {
var calendarDataService = {
getMonths: function() {
return ['January',
'February', // you spelled it "Feburary"
'March',
'April',
'May',
'June',
'July',
'August',
'September',
'October',
'November',
'December'
];
},
getDaysOfWeek: function () {}
return ['Sun', 'Mon', 'Tue', 'Wed','Thu','Fri', 'Sat'];
}
}
// ...
})();
By putting all your language dependent code into one location, you could more easily make your code capable of plugging in a different language, say a Spanish calendar service. All you would need to do to add this feature in the future is to make your module pass it in as a parameter:
var spanishCalendarData = {
getMonths : function () {},
getDaysOfWeek : function () {}
};
(function calendarModule(calendarDataService) {
// ...
})(spanishCalendarData);
Another benefit is that it gives all your code access to the names of the days of the week and months, so any change you make to your calenderDataService
object will change it for your entire module. Also, since the getters return a newly created literal, each call returns a read only copy that the user of the service can manipulate without affecting other users.
var daysOfWeek = calendarDataService.getDaysOfWeek(); // ['Sun', 'Mon', ...]
daysOfWeek.push(daysOfWeek.shift()); // ['Mon', 'Tue', ..., 'Sun']
var another = calendarDataService.getDaysOfWeek(); // still returns ['Sun', 'Mon', ...]
-
\$\begingroup\$ Awesome! Working on my revision. Will post update! \$\endgroup\$user3470505– user34705052016年05月08日 22:48:22 +00:00Commented May 8, 2016 at 22:48
Explore related questions
See similar questions with these tags.
<html>
tag or something? \$\endgroup\$<html>
tag and end with the</html>
tag (or I guess<!DOCTYPE html>
works too, but I normally use<html>
) \$\endgroup\$<html>
. The doctype is not technically part of the HTML document, but tells the browser which rendering mode it should use for the page, e.g. Quirks Mode, Standards Mode, etc. The doctype tag is not an HTML tag. \$\endgroup\$