Javascript and Mongoose for-loop completes before the .find inside is performed

Question

Me and my partner need to get the average rating per movie. Movie and Rating are two separate collections on our MongoDB database.

So, first we need to get all the movies. After that, all we should still need is to iterate in a for-loop through the length of the list of movies returned. In every iteration we Rating.find on the tt_number of the movie that is currently on the index of the for-loop. At that point, it should calculate an average of all of that movie's ratings and store the result in a string JSON. This is done for every movie.

However, the code I included down here does nothing like that. Instead, the for-loop first completes, and then it performs the Rating.find three times afterwards, except with the tt_number of the last iterated movie every time.

var express = require('express');
var mongoose = require('mongoose');
mongoose.connect('mongodb://localhost/Notflix', {useMongoClient: true});
var Movie = require('../model/movies.js');
var Rating = require('../model/ratings.js');
var jwt = require('jsonwebtoken');
var router = express.Router();

router.get('/', function (req, res) {
var jsonString = '{"The average ratings":[]}';
var obj = JSON.parse(jsonString);
var moviett_number;

Movie.find({}, function (err, movies) {
    if (err) {
        res.status(500);
        res.json({errorMessage: '500 SERVER-SIDE ERROR - No list of movies to get the average ratings of could be found.'});
    }

    for (var i = 0; i < movies.length; i++) {
        //TODO: I THINK THIS FOR LOOP IS COMPLETED FULLY BEFORE FINALLY THE RATING.FIND IS DONE???????
        //Go through the list of all movies, getting the ratings for each...
        moviett_number = movies[i].tt_number;

        console.log(i + " - " + moviett_number);

        Rating.find({'tt_number': moviett_number}, function (err, movieRatings) {
            //Get all the ratings for the current movie...

            if (err) {
                res.status(500);
                res.json({errorMessage: '500 SERVER-SIDE ERROR - No list of average ratings for this movie could be found.'});
                return;
            }

            if (movieRatings > 0) {
                //If it has ratings, calculate the average rating.
                var amountOfRatings;
                var average = 0;

                for (amountOfRatings = 0; amountOfRatings < movieRatings.length; amountOfRatings++) {
                    average += parseInt(movieRatings[amountOfRatings].rating);
                }

                average = Math.round((average / amountOfRatings) * 100) / 100;

                //Add the average rating for this movie to the response jsonString.
                obj["The average ratings"].push({
                    averageRatingMessage: 'Movie with tt_number = [' + moviett_number + '] has the following average rating.',
                    averageRating: average
                });
            } else {
                //Add a notice that this movie does not have any ratings and therefore no average rating either to the response jsonString.
                obj["The average ratings"].push({noRatingMessage: 'Movie with tt_number = [' + moviett_number + "] has no ratings yet."});
            }

            //TODO: WATCH FOR THIS CONSOLE.LOG, IT SHOWS A WEIRD ISSUE.
            console.log(obj);
        });
    }

    jsonString = JSON.stringify(obj);

    //TODO: ASYNCHRONOUS ISSUE, RESPONSE IS SET BEFORE THE CODE ABOVE IS DONE, BECAUSE THE PROGRAM CONTINUES EVEN IF THE FOR LOOP ISN'T DONE YET!
    console.log(jsonString);

    res.status(200);
    res.json(jsonString);
});
});

Output:

0 - 123
1 - 456
2 - 789

{"The average ratings":[]}

{ 'The average ratings': [ { noRatingMessage: 'Movie with tt_number = [789] has no ratings yet.' } ] }

{ 'The average ratings': 
   [ { noRatingMessage: 'Movie with tt_number = [789] has no ratings yet.' },
     { noRatingMessage: 'Movie with tt_number = [789] has no ratings yet.' } ] }

{ 'The average ratings': 
   [ { noRatingMessage: 'Movie with tt_number = [789] has no ratings yet.' },
     { noRatingMessage: 'Movie with tt_number = [789] has no ratings yet.' },
     { noRatingMessage: 'Movie with tt_number = [789] has no ratings yet.' } ] }

Update

This question is not a duplicate of the others presented, as this one deals with response builders and dynamic response content. The others are general on how to handle for-loops and tasks to be completed before others. They come close to what I was looking for, but I hadn't found those either while looking for 2 hours straight, plus they just about miss on what I was looking for.

score 0 · Accepted Answer · answered Oct 13 '17 at 20:48

The Model.find() method is asynchronous, so you are sending the result before it is returned from the database. You will need to use a callback once you have fetched all the data to actually send the response.

// callback when all your Rating.find() are complete
function done(movies) {
  res.json(movies);
}

// get the movies
Movie.find({}, function (err, movies) {
  // loop over the results
  movies.forEach(function(movie, i) {
    // get the Rating for this Movie
    Rating.find({'tt_number': movie.tt_number}, function (err, ratings) {
      // add the rating
      movie.rating = ratings.reduce(function(total, rating) {
        total += rating;
        return total;
      }, 0) / ratings.length;

      // this is the last one, call done()
      if (movies.length-1 === i) {
        done(movies);
      }
    });
  }
});

Thank you very much! After some reworking of this code, I got it fully working! You saved me and my partner's skin on this, and we're very thankful for that! — Toastyblast, Oct 14 '17 at 12:16

Jesus Mendoza · Answer 2 · 2017-10-13T21:00:46.327

That's happening because of how Node.js works. It first executes the for loop and it sees a Rating.find to execute, but since Node.js cant execute 2 things at the same time (and its already busy with the for loop) it adds the Rating.find into the a stack to be executed later (when Node.js is free), that happens 3 times in your case because you have 3 movies. When the for loop is done, now Node.js is free to execute the Rating.find but now the value of i (from the for loop) is already 3, that that's why it will always insert the last rating.

To solve this issue (there are multiple ways) you can solve the Rating.find inside an IIFE (Inmediatly Invoked Function Expression). Check below:

var express = require('express');
var mongoose = require('mongoose');
mongoose.connect('mongodb://localhost/Notflix', {useMongoClient: true});
var Movie = require('../model/movies.js');
var Rating = require('../model/ratings.js');
var jwt = require('jsonwebtoken');
var router = express.Router();

router.get('/', function (req, res) {
var jsonString = '{"The average ratings":[]}';
var obj = JSON.parse(jsonString);
var moviett_number;

Movie.find({}, function (err, movies) {
    if (err) {
        res.status(500);
        res.json({errorMessage: '500 SERVER-SIDE ERROR - No list of movies to get the average ratings of could be found.'});
    }

    for (var i = 0; i < movies.length; i++) {
        //TODO: I THINK THIS FOR LOOP IS COMPLETED FULLY BEFORE FINALLY THE RATING.FIND IS DONE???????
        //Go through the list of all movies, getting the ratings for each...
        moviett_number = movies[i].tt_number;

        console.log(i + " - " + moviett_number);

                (function() {
          Rating.find({'tt_number': moviett_number}, function (err, movieRatings) {
              //Get all the ratings for the current movie...

              if (err) {
                  res.status(500);
                  res.json({errorMessage: '500 SERVER-SIDE ERROR - No list of average ratings for this movie could be found.'});
                  return;
              }

              if (movieRatings > 0) {
                  //If it has ratings, calculate the average rating.
                  var amountOfRatings;
                  var average = 0;

                  for (amountOfRatings = 0; amountOfRatings < movieRatings.length; amountOfRatings++) {
                      average += parseInt(movieRatings[amountOfRatings].rating);
                  }

                  average = Math.round((average / amountOfRatings) * 100) / 100;

                  //Add the average rating for this movie to the response jsonString.
                  obj["The average ratings"].push({
                      averageRatingMessage: 'Movie with tt_number = [' + moviett_number + '] has the following average rating.',
                      averageRating: average
                  });
              } else {
                  //Add a notice that this movie does not have any ratings and therefore no average rating either to the response jsonString.
                  obj["The average ratings"].push({noRatingMessage: 'Movie with tt_number = [' + moviett_number + "] has no ratings yet."});
              }

              //TODO: WATCH FOR THIS CONSOLE.LOG, IT SHOWS A WEIRD ISSUE.
              console.log(obj);
          });
        })();
    }

    jsonString = JSON.stringify(obj);

    //TODO: ASYNCHRONOUS ISSUE, RESPONSE IS SET BEFORE THE CODE ABOVE IS DONE, BECAUSE THE PROGRAM CONTINUES EVEN IF THE FOR LOOP ISN'T DONE YET!
    console.log(jsonString);

    res.status(200);
    res.json(jsonString);
});

This is not accurate - Rating.find() is executed but the results are returned asynchronously in a callback. — doublesharp, Oct 14 '17 at 09:50
They are being executed after the foor loop is finished and that's why its always executing with the 789 value. The foor loop is executed and it sees the Rating.find method being called but since it can't execute it because the foor loop is executing and the main thread is busy, the Rating.find call is sent to the stack. That happens 3 times. Now, after the for loop finishes, all the Rating.find calls are executed (one by one, in the order that they were added to the stack) but they are executed with the last value of "i" from the loop because that was the value "i" after the loop finished — Jesus Mendoza, Oct 14 '17 at 21:38
No, they are executing when they are called, but they are *returning* after the loop (and function) have finished. — doublesharp, Oct 14 '17 at 21:39

Javascript and Mongoose for-loop completes before the .find inside is performed

Update

2 Answers2