JQuery/JavaScript : refactoring nested functions

Question

I have this interesting jQuery function. It basically adds a click handler to link, and when that is clicked, it will load a form to allow the user to edit the content. and the form is submitted by AJAX, and will display a success message when it's done.

The outline is below; needless to say, this is messy. I could have each of the callback as a class method. What other ways are there to refactor nested functions? I am also interested to see if there are ways that variables declare in a parent function still retain its value down to the nested function after refactoring

$('a.edit').click( function() {

   // ..snipped..
   // get form
   $.ajax({
       success: function() {

         // add form
         // submit handler for form
         $(new_form).submit(function() {

            // submit via ajax
            $.ajax({

                success: function(data) {
                    // display message
                }
            })

         })

       }}
   )
}

tec · Accepted Answer · 2010-09-29 10:12:19Z

I guess the interesting part of your question is how to refactor without loosing access to the closure variables. Here is my suggestion:

Version one: nested, with closures and variable access:

   var a;
    $('a.edit').click( function() {
      var b;
      $.ajax({
        success: function() {
          var c;
          $(new_form).submit(function() {
            var d;
            $.ajax({
                success: function(data) {
                   // a,b,c,d are all visible here.
                   // note that a references the same object for all calls of the success function, whereas d is a different variable for each call of submit.
                   // this behaviour is called closure: the 'enclosed' function has access to the outer var
                }
            })
          })
        }
      })
    })

Version two: less nested, but without closures and without variable access:

var a;
$('a.edit').click(onEdit);

var onEdit = function() {
  var b;
  $.ajax({success: onEditSuccess});
};

var onEditSuccess = function() {
  var c;
  $(new_form).submit(onSubmit);
};

var onSubmit = function() {
  var d;
  $.ajax({success: onSubmitSuccess});
}

var onSubmitSuccess = function(data) {
  // a is visible (global var)
  // b,c,d NOT visible here.
};

Version three: less nested and with unnamed functions and parameters to get access to the closure variables:

var a;
$('a.edit').click(function(){onEdit(a)});

var onEdit = function(a) {
  var b;
  $.ajax({success: function(){onEditSuccess(a,b)}});
};

var onEditSuccess = function(a,b) {
  var c;
  $(new_form).submit(function(){onSubmit(a,b,c)});
};

var onSubmit = function(a,b,c) {
  var d;
  $.ajax({success: function(data){onSubmitSuccess(data,a,b,c,d)}});
}

var onSubmitSuccess = function(data,a,b,c,d) {
  // a,b,c,d are visible again
  // nice side effect: people not familiar with closures see that the vars are available as they are function parameters
};

lonesomeday · Accepted Answer · 2010-09-29 08:43:08Z

You can easily refactor this to make it much more readable. The key concept to grasp is that you can refer to named functions in callbacks as well as anonymous ones. So, for instance:

function clickHandler() {
    alert("Link clicked");
}
$('a').click(clickHandler);

My preference is always to give the functions names according to what they do (e.g. loadImage, rather than the event that you intend to trigger them (e.g. clickLink. This makes your code clearer and makes later changes much easier. In this case, I would structure my code like this:

$(document).ready(function(){
    $('a.edit').click(loadFormStart);

    function loadFormStart() { // get form
        $.ajax({
            success: loadFormEnd
        });
    }

    function loadFormEnd(data) { // add form & handler
        $('new_form').submit(handleFormStart);
    }

    function handleFormStart() { // submit form
        $.ajax({
            success: handleFormEnd
        });
    }

    function handleFormEnd(data) {  // receive form data
        //display message
    }
});

I'd also advise you to read Code Organization on jqfundamentals which gives a similar approach to this using an object literal.

Mutation Person · Accepted Answer · 2010-09-29 08:29:50Z

0

Interesting question. Personally I don't mind the above. Commenting is key, so you could consider qualifying the closing braces with some:

            } //success: function(data)
        }) //$.ajax({
     }) //$(new_form).submit(

...etc

I would also look at aligning the brackets correctly (at first clance, your }} is a little mystifying).

If it comes to 'generic' nesting strategies, the only other suggestion I have is to move code out other functions. The of course means that you have the function decalred in memory, but may make it more readable.

You could also consider a specific strategy that relates to this code. For example, rather than manually binding a submit to new_form can you use the live function in some way to ensure that it is done automatically?

On a completely unrelated note, you should probably add some ; at the end of each of the bracketed lines!

answered Sep 29, 2010 at 8:29

Mutation Person

30.6k18 gold badges102 silver badges166 bronze badges

2 Comments

lonesomeday Over a year ago

Re using $().live() -- be careful, because the submit event does not bubble in Internet Explorer. Supposedly jQuery fixes this problem, but I found that I had to bind to the click event of the submit button instead, which is obviously not the best solution.

Ruan Mendes Over a year ago

I always stay away from overly nested constructs. Plus any decent IDE obviates the need to document closing brackets, that looks really ugly!

Collectives™ on Stack Overflow

JQuery/JavaScript : refactoring nested functions

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related