3
\$\begingroup\$

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">&lang;</span>
				<span class="left hook"></span>
				<span class="month-year" id="label"></span>
				<span class="right hook"></span>
				<span class="right button" id="prev">&rang;</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>

\$\endgroup\$
4
  • \$\begingroup\$ When I open it, it shows up as unaligned text. Perhaps add an <html> tag or something? \$\endgroup\$
    – user1149
    Commented May 5, 2016 at 14:54
  • 1
    \$\begingroup\$ Wow! I like what you did there. How did you do that? \$\endgroup\$ Commented May 5, 2016 at 15:30
  • \$\begingroup\$ Umm, when you're writing HTML, the document should start with the <html> tag and end with the </html> tag (or I guess <!DOCTYPE html> works too, but I normally use <html>) \$\endgroup\$
    – user1149
    Commented May 5, 2016 at 15:57
  • \$\begingroup\$ @BarryCarter: You are correct in saying the HTML document begins with <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\$ Commented May 5, 2016 at 16:45

1 Answer 1

1
\$\begingroup\$

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', ...]
\$\endgroup\$
1
  • \$\begingroup\$ Awesome! Working on my revision. Will post update! \$\endgroup\$ Commented May 8, 2016 at 22:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.