In Parse.com's Cloud code, asynchronous code is giving variables in for-loop the wrong value

Question

I'm trying to save different food names without duplicates on parse.com. However, when I run the code, the database consists of the same 2 or 3 foods over and over, instead of 200 or so unique names.

Below is my function. I tried logging the name of the food at two different points, and I get different values. The first point gives the correct name of the food, but the second point only shows either flaxseed muffins or raspberry pie. I think the problem has to do with the code running asynchronously, but I'm not sure how to resolve the issue.

Parse.Cloud.define("recordFavorite", function(request, response) {

      var foodList = request.params.foodList; //string array of food names
      var Food = Parse.Object.extend("Food");
      var query = new Parse.Query(Food);

      for (i = 0; i < foodList.length; i++ ) {
              var name = foodList[i];
              console.log("before name is " + name);
              var query = new Parse.Query(Food);
              query.exists("name", name);

              query.find({
              success: function(results) {
                if(results.length == 0){
                  var food = new Food();
                  food.set("name", name);
                  food.save(null, {
                    success: function(food) {
                      console.log("saved with name " + name);
                    },
                    error: function(food, error) {
                    }
                  });
                } else {
                  //don't create new food

                }
              },
              error: function(error) {
              }
            });

      }

});

EDIT:

I was able to make some progress by modifying it to the code pasted below. Now it saves all the objects, including duplicates. I noticed that the lines

var query = new Parse.Query(Food);
query.exists("name", name);

returns an array of all the foods and doesn't filter out the objects containing "name". (To be clear, this was probably still occurring in the original code, but I hadn't noticed.)

Parse.Cloud.define("recordFavorite", function(request, response) {

  var foodList = request.params.foodList; //string array of food names
  var foodListCorrected = new Array();
  var Food = Parse.Object.extend("Food");

  // Wrap your logic in a function
  function process_food(i) {
      // Are we done?
      if (i == foodList.length) {
          Parse.Object.saveAll(foodListCorrected, {
              success: function(foodListCorrected) {

              },
              error: function(foodListCorrected) {

              }
          });
          return;
      }

      var name = foodList[i];
      var query = new Parse.Query(Food);
      query.exists("name", name);

      query.find({
      success: function(results) {
        console.log(results.length);
        if(results.length == 0){
          //console.log("before " + foodListCorrected.length);
          var food = new Food();
          food.set("name", name);
          foodListCorrected.push(food);
        //  console.log(foodListCorrected.length);
        } else {
          //don't create new food
        }
        process_food(i+1)

      },
      error: function(error) {
        console.log("error");
      }
    });

    }

      // Go! Call the function with the first food.
      process_food(0);


    });

Answer 1

I believe this (https://www.parse.com/docs/js_guide#promises-series) is the solution you're looking for. You need to utilize promises to force synchronicity.

Answer 2

I think you're right about the problem being the async logic. The problem is that the outer loop completes as quickly as it can, firing off the various, slower async calls for your food lookup queries as it goes. The outer loop doesn't wait and because of what's know as 'variable hoisting' when you access 'name' inside your success function, its value will be the latest value of 'name' in the outer loop. So when the success function is called, the value of name has moved on to a different food to when you first initiated the exists/save query sequence.

Here's a really simple example:

Say your foodList looked like ['Muffin'], ['Cheesecake']. When you enter the loop for the first time, you have name='Muffin'. You fire off your exists query for name='Muffin' and that now happens asynchronously. Meanwhile, the outer loop happily moves on and sets name='Cheesecake' and fires off another exists query. Meanwhile. your first exists query completes and you are now ready to save the first food. But, because of hoisting, the value of name within your success function is now 'Cheesecake'. So it saves 'Cheesecake' when it should have saved 'Muffin' Then the second set of async queries complete, and this one also saves 'Cheesecake'. So you get two foods, representing your two unique foods, but both are called 'Cheesecake'!

Here's the classic article on variable hoisting, it is well worth a read:

http://www.adequatelygood.com/JavaScript-Scoping-and-Hoisting.html

A way of solving this would be to only trigger the processing of the next food once all the async calls for the current food have completed. You can do this like this:

Parse.Cloud.define("recordFavorite", function(request, response) {

  var foodList = request.params.foodList; //string array of food names
  var Food = Parse.Object.extend("Food");
  var query = new Parse.Query(Food);

  // Wrap your logic in a function
  function process_food(i) {
      // Are we done?
      if (i == foodList.length) return;

      var name = foodList[i];
      console.log("before name is " + name);
      var query = new Parse.Query(Food);
      query.exists("name", name);

      query.find({
      success: function(results) {
        if(results.length == 0){
          var food = new Food();
          food.set("name", name);
          food.save(null, {
            success: function(food) {
              console.log("saved with name " + name);
              // Move onto the next food, only after all the async operations
              // have completed.
              process_food(i+1)
            },
            error: function(food, error) {
            }
          });
        } else {
          //don't create new food

        }
      },
      error: function(error) {
      }
    });
  }

  // Go! Call the function with the first food.
  process_food(0);
});

(Note, I've not tested this code, so there might be syntax errors).

I've not come across Parse before... I saw your question, went off to read about it, and thought it looked very interesting! I will remember it for my next PHP API project. I think there are some smarter things you can try to do. For example, your approach requires 2 async calls per food, one to see if it exists, and one to save it. For 200 foods, that's 400 async calls. However, the Parse API looks very helpful, and I think it will offer tools to help you cut this down. You could probably try something along the following lines:

You already have an array of strings of the names you want to save:

var foodList = request.params.foodList; //string array of food names

Say it looks like ["Cupcakes", "Muffins", "Cake"].

Now build a Parse query that gets all food names already on the server. (I don't know how to do this!). But you should get back an array, let's say ["Cupcakes", "Cheesecake"].

Now you an strip the duplicates in JavaScript. There'll be some nice questions here on StackOverflow to help with this! The result will be that "Cupcake" is a duplicate, so we are left with the array ["Muffins", "Cake"]

Now it looks like in Parse you can Batch some operations:

https://parse.com/docs/rest#objects-batch

so your goal is to save this array of ["Muffins", "Cake"] with one API call.

This approach will scale well with the number of foods, so even with 200 foods, you should be able to do it in one query, and one batch update per 50 foods (I think 50 is a batch limit, from the Parse docs), so at most you will need 5 API calls.