How to properly use callback in Node.JS?

Question

Node.js

I am writing an asynchronous image loader, it was necessary to read the hash of the file after loading. Can't implement
Here is the program code:

var images = {};
        for (var i in news.items) {
            if(typeof news.items[i].attachments !== undefined) {
                for (var j in news.items[i].attachments)
                    if (news.items[i].attachments[j].type == 'photo') {
                        download(news.items[i].attachments[j].photo.photo_604, 'tmp/' + url.parse(news.items[i].attachments[j].photo.photo_604).pathname.split('/').pop(), function() {
                            fs.readFile('tmp/' + url.parse(news.items[i].attachments[j].photo.photo_604).pathname.split('/').pop(), function(err, data) {
                                console.log(checksum(data, 'sha1'));
                            });
                        });
                    }
            }
        }

Here is the code for the download and checksum functions

var download = function(url, dest, cb) {
  var file = fs.createWriteStream(dest);
  https.get(url, function(response) {
    response.pipe(file);
    file.on('finish', function() {
      file.close(cb);  // close() is async, call cb after close completes.
    });
  });
};

function checksum (str, algorithm, encoding) {
    return crypto
        .createHash(algorithm || 'md5')
        .update(str, 'utf8')
        .digest(encoding || 'hex')
}

Random hashes are displayed in the console, and if you try to print i or j, they are constant and do not change. How to fix this problem?

Answer 1

If you want to leave this lovely nesting in the code, then you can do this

var images = {};
        for (var i in news.items) {
            if(typeof news.items[i].attachments !== undefined) {
                for (var j in news.items[i].attachments)
                    if (news.items[i].attachments[j].type == 'photo') {
                        download(news.items[i].attachments[j].photo.photo_604, 'tmp/' + url.parse(news.items[i].attachments[j].photo.photo_604).pathname.split('/').pop(), function(i, j) {
                            fs.readFile('tmp/' + url.parse(news.items[i].attachments[j].photo.photo_604).pathname.split('/').pop(), function(err, data) {
                                console.log(checksum(data, 'sha1'));
                            });
                        }.bind(i, j));
                    }
            }
        }

Or like this:

var images = {};
Object.keys(news.items).forEach(function(key) {
    var item = news.items[key];
    if (!Array.isArray(item.attachments)) return;

    item.attachments.forEach(function(attachment) {
        it (attachment.type !== 'photo') return;

        var loc_path = 'tmp/' + url.parse(attachment.photo.photo_604).pathname.split('/').pop();
        download(attachment.photo.photo_604, loc_path, function() {
            fs.readFile(loc_path, function(data) {
                console.log(checksum(data, 'sha1'));
            });
        });
    });
});

This is the minimum that is needed to make the code readable.
Further, I cannot understand why you first download the file to the file system, and then load it into memory, why not immediately load it into memory.
I also recommend using promises instead of callbacks - this will make the code even more readable.

Answer 2

It's all about asynchrony. When the callback executes (the next tick of the event loop), the loop has already ended and i and j have reached their last value.
You need promises.

Answer 3

Don't forget about async and use https://www.npmjs.com/package/bluebird