2
\$\begingroup\$

I often use jQuery plugins for front-end functionality in web applications. This requires me to write code like this, where I am assigning a function to an object property:

var trainingDiary = $("#training_diary").fullCalendar({
  dayClick: function(date, allDay, jsEvent, view) {

    var title = prompt('Event Title:');
    var dayObj = $(this);
    //console.log($(this));
    if (title) {
     $.ajax({
      url: "/app_dev.php/addtrainingday",
      global: false,
      type: "POST",
      data: "dayNo=" + date.getDate(), //returns day of month. getDay() returns day of week
      async:true,
      success: function(msg) {

               trainingDiary.fullCalendar('renderEvent',
                {
                  title: title,
                  start: date,
                  allDay: allDay,
                  backgroundColor:'#cccccc'
                },
            true // make the event "stick" across calendar pages
          )

      dayObj.css({'background-color':'#339933','background-image':'url()'})
      console.log(msg.valueOf());
      } //end ajax success
     })
   } else {
      trainingDiary.fullCalendar('unselect');
   }

  },

  theme: true,
  header: {left:'title',
            center: '',
            right: 'today prev next'},
  firstDay:1

});

The code becomes hard to read and maintain as there is now a success function within an Ajax function within yet another function. How can I improve the code so that dayClick: function(...) can call a function defined outside of the fullCalendar call? In this case, fullCalendar is a jQuery plugin.

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

You should use the JavaScript Module Pattern to create a private scope for your function declarations:

// Directives for JSLint
/*global console, prompt, $ */ // declaration of global variables used
/*jslint unparam: true, sloppy: true, white: true */ // disable less useful checks

// create a closure for private scope
(function(){

  // Private declarations

  var calendarOptions, // object, options for calendar diary
      trainingDiary;   // object, training diary using jQuery plugin fullCalendar()

  function onDayClick(date, allDay, jsEvent, view) {
    // Function: onDayClick(day, allDay, jsEvent, view)
    // Callback for the selection of a day in the calendar.
    //
    // Parameters:
    //   date - type?, description?
    //   allDay - type?, description?
    //   jsEvent - type?, description?
    //   view - type?, description?

    // single var declaration
    var title,       // string, event title, input by user
        dayObj,      // object, jQuerified DOM element
        ajaxOptions; // object, options for AJAX call to addtrainingday script

    // this declaration must be nested to access date, allDay... in closure scope
    function onAjaxSuccess(msg) {
      // Function: onAjaxSuccess(msg)
      // Callback for a successful AJAX call to server-side addtrainingday script.
      //
      // Parameter:
      //   msg - string, message received from server, for logging purpose        

      var event = {
        title: title,
        start: date,
        allDay: allDay,
        backgroundColor:'#cccccc'
      };

      trainingDiary.fullCalendar(
        'renderEvent',
        event,
        true // make the event "stick" across calendar pages
      ); // added missing semicolon

      dayObj.css({
        'background-color':'#339933',
        'background-image':'url()'
      }); // added missing semicolon

      console.log(msg.valueOf());

    } // end of onAjaxSuccess declaration

    // Init
    title = prompt('Event Title:');
    dayObj = $(this);
    ajaxOptions = {
      url: "/app_dev.php/addtrainingday",
      global: false,
      type: "POST",
      // getDate() returns day of month. getDay() returns day of week
      data: "dayNo=" + date.getDate(),
        async: true,
        success: onAjaxSuccess
    };

    //console.log($(this));
    if (title) {
      $.ajax(ajaxOptions); // added missing semicolon
    } else {
      trainingDiary.fullCalendar('unselect');
    }

  } // end of onDayClick declaration

  // Initialization code

  calendarOptions = {
    dayClick: onDayClick,
    theme: true,
    header: {
      left: 'title',
      center: '',
      right: 'today prev next'
    },
    firstDay: 1
  };

  trainingDiary = $("#training_diary").fullCalendar(calendarOptions);

}()); // the function is called immediately to make the declarations effective

I also created extra variables to hold options objects, to enhance readability, and I added comments to describe the intent and parameters of functions declared.

Note that trainingDiary is now declared in the private scope as well. If you need to access it in the global scope, you will need to export it to the global object by assigning it to global this or window:

(function(){

  // Private declarations
  (...)

  // Init
  (...)

  // Public declarations
  this.trainingDiary = trainingDiary;
}());

I checked the code with latest JSLint; it is a useful tool once you know what to expect from it, e.g. by reading "JavaScript: The Good Parts" from its author Douglas Crockford. It would definitely help you to spot the missing semicolons, which can cause unexpected issues when you minify your code.

\$\endgroup\$
4
  • \$\begingroup\$ Wow - thanks so much for such a comprehensive answer. I will take a look into this approach and report back. Can you explain how one would '[assign it to] global this'? \$\endgroup\$
    – codecowboy
    Commented Jul 30, 2011 at 5:10
  • 1
    \$\begingroup\$ This is shown in second example: this.trainingDiary = trainingDiary; A classic alternative would be to assign to window, which only works in a browser environment: window.trainingDiary = trainingDiary; \$\endgroup\$
    – user185
    Commented Jul 30, 2011 at 12:21
  • \$\begingroup\$ yep - my bad. I realised this after I read through your answer again and the link on closures. I've updated my question with the latest code. JSLint complains about ROOT_URL - this is defined in another file. \$\endgroup\$
    – codecowboy
    Commented Jul 30, 2011 at 16:48
  • \$\begingroup\$ my bad again - just realised I can alert JSLint to an additional global declaration and you had already done this for jquery \$\endgroup\$
    – codecowboy
    Commented Jul 30, 2011 at 17:06

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.