How to avoid spaghetti code in Javascript [closed]

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance. Closed 9 years ago.

I'm f开发者_运维百科inding myself writing a lot of spaghetti in Javascript when I have to deal with asynchronous applications (specially when dealing with OpenSocial code where all the data has to be obtained through JS). The usual pattern is something like:

1. User logs into the application for the first time, get his data.
2. Do A on his data (e.g. get his friends by sending a request to the server).
3. Do B on this data (e.g. send his friends to the server for some processing).
4. Do C on his data (e.g. check that the server response is valid so we can do something else).

Note that this sequential execution path (1 => 2 => 3 => 4) doesn't fit well the async. nature of Ajax so the user ends up waiting for a long time and the code turns into a mess since every step depends on the previous ones.

An example with code:

gadgets.util.registerOnLoadHandler(setupUser())
...
function setupUser() {
  var req = [get data and setup request]
  req.send(some_url, some_data, function(response) { getFriendsFor(response.user) });
}

function getFriendsFor(user) {
  var friends = [get friends from user]
  var req = [setup request] 
  req.send(some_other_url, some_other_data, function(response { validateFriendsResponse(response.friends) });
}

function validateFriendsResponse(friends) {
  if (friends.valid())
    ...
  loadCanvas();
}

You can see that each function depends on the previous one, and what's worse, it has to be called in a specific order to be useful. It gets worse when you have to add stuff like showing/hiding loading screens and other gimmicks while the user waits.

How would you go about fixing this?

---

One option might be to have a variable that shows the current state, and have a "controller" function that is always the AJAX callback function. Based on the current state, the controller function will call the next function in line. To simplify the controller function, I'd probably store the sequence of functions to call in a Javascript object, so all the controller function is doing is a lookup and a pass off to the next function in the sequence. This approach might be facilitated by having a single Javascript object that is always the parameter to the function (and contains all of the data that was returned by earlier AJAX calls.

Example:

var user = {};
var currentState = 1;

var someFunction = function(user) {//stuff here that adds data to user via AJAX, advances currentState, and calls controllerFunction as callback};
var someOtherFunction = function(user) {//stuff here that does other things to user, advances currentState, and calls controllerFunction as callback}

var functionSequence = {1:someFunction, 2:someOtherFunction}

var controllerFunction = function() {
   //retrieve function from functionSequence based on current state, and call it with user as parameter 
}

---

You can control this sort of spaghetti with things like the Observer pattern. Some JavaScript frameworks have a ready-to-use implementation of this functionality, for example Dojo's publish/subscribe functions.

---

Code to handle the showing, hiding, and status features should be extracted into functions. Then, to avoid the "spaghettiness", one solution is to use the anonymous functions inline.

function setupUser() {
  var req = [get data and setup request]
  req.send(some_url, some_data, function(response) { 
    var friends = [get friends from user]
    var req = [setup request] 
    req.send(some_other_url, some_other_data, function(response {         
      if (friends.valid())
      ...
      loadCanvas();
    });
  });
}

---

Part of the problem is that you're thinking of this as a four-step process with three round trips to the server required. If you really think this is a single workflow, and the thing the user is most likely to do, then the best speedup is to collect as much info in the first interaction as possible to reduce the round trips. This may include allowing the user to check a box saying she wants to follow this path, so you don't have to return to the user in between steps, or allowing her to enter a guess at the friends' names that you can process the first time around, or pre-loading the name list the first time.

The way you've outlined the code works best if this is just one of many paths the user might follow; the multiple round trips are required because at each interaction, you're finding out what the user wants, and a different answer would have sent you in a different direction. This is when the loosely coupled code style you are disparaging really shines. It looks like each step is disconnected from what went before because the user's actions are driving the activity.

So, the real question is whether the user has a choice at the beginning that determines the direction your code goes. If not, then you want to optimize the path you know (or strongly predict) the interaction is going to go. On the other hand, if the user interactions drive the process, then decoupling the steps and reacting to each interaction is the right thing, but you'd expect many more divergent possibilities.

---

Build your javascript client with an MVC architecture

• Backbone (documentcloud.github.com/backbone)

• Spine (spinejs.com)

• JavascriptMVC - http://www.javascriptmvc.com/

• SproutCore - http://www.sproutcore.com/

• Jamal - http://jamal-mvc.com/

• TrimJunciton - http://code.google.com/p/trimpath/wiki/TrimJunction

• Trimpath - http://code.google.com/p/trimpath/

---

If you want your functions to be capable of operating independently, equip the asynchronous ones with generic callbacks instead of calls to the "next" function in line. Then, as JacobM said, set up a "controller" that will call them in sequence. I've modified your sample code below to demonstrate (beware, this hasn't been tested):

gadgets.util.registerOnLoadHandler(userSetupController())
...
function setupUser(callback) {
  var req = [get data and setup request]
  req.send(some_url, some_data, function(response) { callback(response.user) });
}

function getFriendsFor(user,callback) {
  var friends = [get friends from user]
  var req = [setup request] 
  req.send(some_other_url, some_other_data, function(response { callback(response.friends) });
}

function validateFriendsResponse(friends) {
  if (friends.valid())
    return true;
  else
    return false;
}

function userSetupController() {
    setupUser(function(user){
        getFriendsFor(user,function(friends){
            if (validateFriendsResponse(friends)) {
                loadCanvas();
            } else {
                // don't load the canvas?
            }
        });
    });
}

Creating callbacks get a little tricky if you're not familiar with them - here's a decent explanation: http://pixelpushing.net/2009/04/anonymous-function-callbacks/. If you want to get more complex (again, as JacobM suggested), you could write some code that handles this automatically - give it a list of functions, and it executes them in order, passing the callback data around. Convenient, but may be overkill for your needs.