JavaScript: Accessing variable inside callback

Question

I'm trying to use the open-graph plugin in NodeJS to get a preview image for a vine. The OG result is correct, but I can't access result[i] from within the og callback - the variable is undefined. How can I access result[i] in the OG callback?

    Thing.find(function(err, result) {
        for (var i = 0; i < result.length; i++) {
            if (result[i].attachment) {
                if (result[i].attachment.embed_type == 'vine') {
                    og(result[i].attachment.embed_url, function(err, meta) {
                        result[i].attachment.preview_image = meta.image;
                        result[i].save();
                    });
                }
            }
        }
    });

possible duplicate of JavaScript closure inside loops – simple practical example — Zirak
– Zirak, Commented Apr 3, 2015 at 23:39

adeneo · Accepted Answer · 2015-04-03 16:56:40Z

1

You need a closure, i keeps changing because og is async

Thing.find(function(err, result) {
    for (var i = 0; i < result.length; i++) {
        if ( result[i].attachment &&
             result[i].attachment.embed_type == 'vine') 
        {
            (function(res) {
                og(res.attachment.embed_url, function(err, meta) {
                    res.attachment.preview_image = meta.image;
                    res.save();
                });
            }(result[i]));
        }
    }
});

edited Apr 3, 2015 at 16:56

answered Apr 3, 2015 at 16:55

adeneo

319k29 gold badges409 silver badges392 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

Sethen Over a year ago

This should be done outside the for loop for clarity. An IIFE is not the way to go here.

adeneo Over a year ago

In my opinion an IIFE is the way to go here, but any function will do

Sethen Over a year ago

It's not extendable and really just complicates things further. This is not correct and doesn't help the clarity and reusability of this code at all.

adeneo Over a year ago

Why is it not correct, and why would it complicate things ?

Sethen Over a year ago

What happens when I want to unit test that function separately? I should be allowed to do this easily by simply calling the function. You're also forcing other engineers to find the for loop this resides in if they want to make changes. The reason this should be in a function outside the for loop is the same reason why we use different classes to extend from. This isn't correct in my opinion and complicates matters further. While it may work, it's messy and foolish.

|

Community · Accepted Answer · 2017-05-23 12:22:11Z

JS does not have block scope.

There are some ways to work around this problem, like creating a function, for example.

And, to make it more clear and easily testable, I would put all the result[n] logic in the other function as well.

var process = function(resultItem) {
   if (resultItem.attachment && if (resultItem.attachment.embed_type == 'vine')) {
     og(resultItem.attachment.embed_url, function(err, meta) {
       resultItem.attachment.preview_image = meta.image;
       resultItem.save();
     });
   }
};

Thing.find(function(err, result) {
  for (var i = 0; i < result.length; i++) {
     process(result[i]);
  }
});

Collectives™ on Stack Overflow

JavaScript: Accessing variable inside callback

2 Answers 2

7 Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

7 Comments

Comments

Linked

Related