2
function inboxUsers(){
    for (var i=0; i<uniqueArray.length; i++){
        var getUsername    = 'SELECT userName FROM users WHERE userId = ' + uniqueArray[i];
        db.query(getUsername, function(err, results) {
            if (err) {
                console.log('Error in database');
                throw err;
            }
            for(var i in results){
                console.log('single',results[i].userName);
                inboxUserList.push(results[i].userName);
            }
        });
    }
    sample();
}

function sample(){
    console.log('same function');
}

This is my console output.

same function
single user1
single user2
single user3

In this code I called the function sample() after for loop, but it called the sample() function before for loop ends.

I want to call the sample() function when for loop ends. I am a beginner for stackoverflow, if i have error please apologize me. Thank you

yathavan
  • 2,051
  • 2
  • 18
  • 25
  • Welcome to StackOverfow! Please update your question to include the output to the console or otherwise describe how you know that function sample is being called before the loop ends. – dgvid Apr 16 '15 at 17:42
  • 3
    The `for` loop will always be complete before `sample` runs; `db.query`, however, appears to be asynchronous, meaning that `sample` will run before the callback you have passed to it. – Andrew Larson Apr 16 '15 at 17:44
  • 1
    @dgvid I updated my question. these include console output – yathavan Apr 16 '15 at 17:54
  • See my answer below. This is a very common asynchronous scenario that can be easily fixed :) – CatDadCode Apr 16 '15 at 18:23

2 Answers2

5

Your call to db.query is asynchronous. What that means is this:

  1. The call to db.query(...) returns immediately, returning nothing.

  2. Instead of assigning a return value to a variable (var results = db.query(...)), you pass in a callback function as an argument so that the db module can call when it's done fetching its results. It will hang onto the callback function until the database has your results, then it will call the function when it's ready.

  3. Because the call to db.query(...) returns immediately, your for loop will complete and the call to sample() will fire before the callback functions you supplied to your queries are ever called by the db module.


To ensure that sample runs when all of your calls are done, you'll need to track the completion of each query and then fire the sample function when all of the queries have returned. In my opinion, the easiest way to do this without introducing you to complex topics like "promises", is with a module called async and its parallel method.

$ npm install async --save

var async = require('async');
var queries = [];

function inboxUsers(){
  uniqueArray.forEach(function (userId) {
    var getUsername = 'SELECT userName FROM users WHERE userId = ' + userId;
    queries.push(function (done) {
      db.query(getUsername, done);
    });
  });
  async.parallel(queries, function (err, allQueryResults) {
    if (err) { return console.error(err); }
    allQueryResults.forEach(function (queryResults) {
      queryResults.forEach(function (result) {
        console.log('single', result.userName);
        inboxUserList.push(result.userName);
      });
    });
    sample();
  });
}

function sample(){
  console.log('same function');
}

Here it is again but with fewer shortcuts taken and detailed comments.

var async = require('async');

// create an array to store a bunch of functions that the async library
// should fire and wait to finish.
var queries = [];

function inboxUsers(){
  uniqueArray.forEach(function (userId) {
    var getUsername = 'SELECT userName FROM users WHERE userId = ' + userId;
    var queryFunc = function (done) {
      db.query(getUsername, function(err, results) {
        // let the async lib know this query has finished.
        // the first argument is expected to be an error.
        // If the err is null or undefined then the async lib
        // will ignore it. The second argument should be our results.
        done(err, results);
      });

      // You could make the above even simpler by just passing
      // the done function as the callback to db.query. I just
      // didn't want to confuse you by doing that.
      // db.query(getUsername, done);
    };
    queries.push(queryFunc);
  });
  // Fire all async functions by passing in our queries array.
  // The async library will wait for them all to call "done()" 
  // before it invokes this final function below.
  async.parallel(queries, function (err, allQueryResults) {
    // If any of our queries pass an error to "done" then the async
    // lib will halt the rest of the queries and immediately invoke
    // this function, passing in the error.
    if (err) { return console.error(err); }

    // queryResults is an array containing the results of each query we made.
    allQueryResults.forEach(function (queryResults) {
      queryResults.forEach(function (result) {
        console.log('single', result.userName);
        inboxUserList.push(result.userName);
      });
    });

    // All your queries are complete and your inboxUserList array
    // is populated with the data you were after. Now we can call
    // "sample".
    sample();
  });
}

function sample(){
  console.log('same function');
}

The async lib knows how many functions you supplied to the array so it knows how many calls to done it should wait for before invoking the final function.

Community
  • 1
  • 1
CatDadCode
  • 58,507
  • 61
  • 212
  • 318
  • this cod push same username in array, I print inboxUserList in sample(), I got this result `[ 'kabilan', 'kabilan', 'kabilan' ]` – yathavan Apr 16 '15 at 18:31
  • I also changed the `for (var i=0; i – CatDadCode Apr 16 '15 at 18:40
  • `console.log('single', result.userName);` this line print **Undefiend** – yathavan Apr 16 '15 at 19:01
  • I can't identify anymore errors in my sample, and I can't test it since I don't have the full code you're running. I'd start by inspecting what is getting passed to the `async.parallel` callback (`allQueryResults`) and see what that looks like. It should be an array of arrays. Each nested array being the results of the relevant query. Try `console.log(result);` also to see what `result` is. If it's not what you expect it to be then we can work from there to find out why. – CatDadCode Apr 16 '15 at 19:10
1

You are most likely experiencing this problem because your db.query() function is not synchronous. It expects a callback function that it can invoke when it's finished.

Your code inboxUserList.push(...) will not be called until your database library queries the database and gets a result. Meanwhile, your for loop will continue running, prepping all of your queries and continuing before they all finish. Then sample() is called, because the for loop is done, even if the callbacks you passed in have not been called yet.

There are many solutions, but perhaps the simplest with your current code would be something like this:

function inboxUsers(){
    var completed = 0;
    for (var i=0; i<uniqueArray.length; i++){
        var getUsername    = 'SELECT userName FROM users WHERE userId = ' + uniqueArray[i];
        db.query(getUsername, function(err, results) {
            if (err) {
                console.log('Error in database');
                throw err;
            }
            for(var i in results){
                console.log('single',results[i].userName);
                inboxUserList.push(results[i].userName);
            }

            completed++;
            if (completed == uniqueArray.length) {
                sample();
            }
        });
    }
}

function sample(){
    console.log('same function');
}
CatDadCode
  • 58,507
  • 61
  • 212
  • 318
Zeimyth
  • 1,389
  • 12
  • 19
  • I have got this error, `TypeError: undefined is not a function at inboxUsers (C:\Users\yathu\IdeaProjects\chatting\app.js:148:16)` , This is a line 148 : **}).then(function() {** – yathavan Apr 16 '15 at 18:15
  • @user3702886 What library are you using for db? – Zeimyth Apr 16 '15 at 18:16
  • var mysql = require('mysql'); – yathavan Apr 16 '15 at 18:22
  • @user3702886 Change `.then(function()` to `.on("end", function()`, according to https://www.npmjs.com/package/mysql#streaming-query-rows – Zeimyth Apr 16 '15 at 18:27
  • 1
    You don't even need to use `.then` or `.on` in this scenario. Simply move the `completed++` and the following `if` statement up into the query callback function below the for loop. Then you can avoid promises and event handlers altogether, which are somewhat complex topics for a beginner. – CatDadCode Apr 16 '15 at 18:28
  • 1
    @AlexFord That's a great point! I simplified my example to reflect that. – Zeimyth Apr 16 '15 at 18:32
  • Nice job. This is basically doing what the `async` library in my answer is doing, by tracking each query as it completes. Great example :) – CatDadCode Apr 16 '15 at 18:45
  • @user3702886 Just keep in mind that this lacks the error handling that the `async` library does for you. When you pass an error to the `done` callback in my answer, the async lib will stop invoking your queries and immediately jump to your result handler and pass in the error. You'll need to do all that short-circuiting manually in this setup. Just something to be aware of incase it bites you later. That's the only significant difference between doing the query tracking manually and using a library :) – CatDadCode Apr 16 '15 at 18:54