NodeJS http.get() async and callback function

Question

I am working on the "learnyounode" practice 9, Juggling Async. The problem states:

This problem is the same as the previous problem (HTTP COLLECT) in that you need to use http.get(). However, this time you will be provided with three URLs as the first three command-line arguments.
You must collect the complete content provided to you by each of the URLs and print it to the console (stdout). You don't need to print out the length, just the data as a String; one line per URL. The catch is that you must print them out in the same order as the URLs are provided to you as command-line arguments.

I have two different solutions which are mostly the same. The only difference is that the working one is modularized while the other is not (and does not work). I am having trouble understanding the differences and why. Here are my code:

Working solution:

const http = require('http');
const bl = require('bl');

var msgs = [];
var complete = 0;

function printAll() {
    for (var i = 0; i < 3; i++) {
        console.log(msgs[i]);
    }
}

function doGet(i) {
    http.get(process.argv[i + 2], function(res) {
        res.pipe(bl(function(err, data) {
            if (err) {
                return console.error(err);
            }
            complete++;
            msgs[i] = data.toString();
            if (complete == 3) {
                printAll();
            }
        }));
    });
}

for (var i = 0; i < 3; i++) {
    doGet(i);
}

Wrong solution singleFunction.js:

const http = require('http');
const bl = require('bl');

var msgs = [];
var complete = 0;

for (var i = 0; i < 3; i++) {
    http.get(process.argv[i + 2], function(res) {
        res.pipe(bl(function(err, data) {
            if (err) {
                return console.error(err);
            }
            complete++;
            msgs[i] = data.toString();
            if (complete == 3) {
                for (var j = 0; j < 3; j++) {
                    console.log(msgs[j]);
                }
            }
        }));
    });
}

Output of learnyounode run singleFunction.js:

undefined
undefined
undefined

I am not even sure what are the things I do not fully understand here. Is it NodeJS event loop, or passing Javascript functions, or NodeJS callbacks, or even something else? Thank you for the help.

EDIT: Finally figured out the reason and fix with the helpful comments. The reason modularized solution works is because scope of i in doGet(i) as well as the callback are not the same as the for-loop. Therefore the index value is correct. The main for-loop would finish and increment i to 3 way before any callback is invoked, hence getting undefined at index 0,1,2.

You're declaring a local `i` inside the `bl()` callback function. That will hide the more global `i`. Even without that, you'd run into the common problem of trying to use a loop index variable in a callback constructed in the loop. — Pointy, May 04 '16 at 17:27
in the "wrong" solution, try to print the value of msgs[3] instead, it will store the value of your last executed callback. The reason is function scope visibility of the `i` in the outer loop — Yerken, May 04 '16 at 17:34
in the first "correct" solution you create a closure, hence value of `i` is preserved, again, because it has a function scope visibility. — Yerken, May 04 '16 at 17:36
@Yerken, printing msgs[3] gave me the same message three times, not "undefined" anymore. — derrdji, May 04 '16 at 17:42
@derrdji exactly, you need to understand how variable scopes work in javascript — Yerken, May 04 '16 at 17:43
in other words this is a dupe of "For Loop closure: A Simple Practical Example" — Kevin B, May 04 '16 at 18:24

NodeJS http.get() async and callback function

0 Answers0