Trouble returning response from asynchronous call

Question

Following this fiddle from this question, I wrote this bit of code:

var currentSlideCount = window.imgIds.length;

for (var i = 11; i < (currentSlideCount + 10); i++) {

// SET UP NEW SLIDE HTML
    var newSlide = '<li><img id="apod' + i + '" class="rounded-corners apod-image"></li>';
    $('#lightSlider').append(newSlide);
    window.imgIds.push('apod'+i);
    console.log(window.imgIds);

// GENERATE DATE
    var date = new Date();
    date.setDate(date.getDate() - i);
    var day = date.getDate();
    var month = date.getMonth();
    var year = date.getFullYear();
    console.log(year + "-" + month + "-" + day);

// GENERATE XML REQUEST
    function foo(callback) {
        var apodUrl = "https://api.nasa.gov/planetary/apod?concept_tags=True&date=" + year + "-" + month + "-" + day;
        var apodXml = new XMLHttpRequest();
        apodXml.open('GET', apodUrl, true);
        apodXml.send(null);

        // WHEN REQUEST IS READY
        apodXml.onreadystatechange=function() {
            if (apodXml.readyState==4 && apodXml.status==200) {
                var apodParse = JSON.parse(apodXml.responseText);
                callback(apodParse.url)
                console.log(apodParse.url);
            }
        }
    }

    foo(function(result) {
        var newSlideId = 'apod' + i;
        document.getElementById(newSlideId).src = result;
    });

Yet I'm still getting a Cannot set property 'src' of null console error on the img tag that was created long before its src attribute is being called on. And as far as I understand, I've set up the callback correctly. Why is this still not working?

Answer 1

First, you are declaring the function foo in a loop. While this is not leading to an error, it's bad practice. The function should be declared outside of the loop.

Second, the callback function passed into foo is invoked asynchronously (i.e., via AJAX). The variable i is assigned a value in the parent scope of the callback function and in a loop. The loop will have finished executing by the time the callback is invoked. When the callback is invoked, it will look up the scope chain to find the value of i and it will find i declared in the loop. i will be equal to the final value in the loop for which the loop conditional i < (currentSlideCount + 10) evaluates false and does not continue.

While this may be hard to follow, you can see what I mean by adding alert(i); to the callback function:

 foo(function(result) {
    alert(i);
    var newSlideId = 'apod' + i;
    document.getElementById(newSlideId).src = result;
 });

You may be surprised to see that the alert will always display the same value for i.

To solve this problem, you need to use an immediately executing function to create a new scope where i is passed by value as the proper value you want.

Change this:

foo(function(result) {
    var newSlideId = 'apod' + i;
    document.getElementById(newSlideId).src = result;
});

To this:

foo(
    (function(i) {
        return function(result) {
            var newSlideId = 'apod' + i;
            document.getElementById(newSlideId).src = result;
        }
    })(i)
);

In JavaScript, scope is delineated at the function level. By using an immediately executing function, you are adding a new scope where i has been passed by value for the current iteration of the loop.

Variable scoping in JavaScript can be tricky to understand and your question hits right at one of the more complex scenarios. You may find it useful to review some other explanations of scoping in JavaScript.

Answer 2

Two problems there:

1. You're using a function declaration inside a control structure. You can't do that, it's invalid; some browsers will try to rewrite it for you as a function expression, but others will not. Function declarations are only valid at the top level of a scope, outside of all control structures. E.g., at global scope or at the top level of a function.

2. More importantly, the callback you're passing to foo has an enduring reference to the i variable, not a copy of it as of when the function was created. So they all see i as it is when they run, later, at the end of the loop.

Move your foo function out of the control structure, and ideally parameterize it, in particular passing it the i value that the callback should use. E.g.:

var currentSlideCount = window.imgIds.length;

for (var i = 11; i < (currentSlideCount + 10); i++) {

    // SET UP NEW SLIDE HTML
    var newSlide = '<li><img id="apod' + i + '" class="rounded-corners apod-image"></li>';
    $('#lightSlider').append(newSlide);
    window.imgIds.push('apod' + i);
    console.log(window.imgIds);

    // GENERATE DATE
    var date = new Date();
    date.setDate(date.getDate() - i);
    var day = date.getDate();
    var month = date.getMonth();
    var year = date.getFullYear();
    console.log(year + "-" + month + "-" + day);

    foo(year, month, day, i, function(result, index) {
        var newSlideId = 'apod' + index;
        document.getElementById(newSlideId).src = result;
    });
}

// GENERATE XML REQUEST
function foo(year, month, day, index, callback) {
    var apodUrl = "https://api.nasa.gov/planetary/apod?concept_tags=True&date=" + year + "-" + month + "-" + day;
    var apodXml = new XMLHttpRequest();
    apodXml.open('GET', apodUrl, true);
    apodXml.send(null);

    // WHEN REQUEST IS READY
    apodXml.onreadystatechange = function() {
        if (apodXml.readyState == 4 && apodXml.status == 200) {
            var apodParse = JSON.parse(apodXml.responseText);
            callback(apodParse.url, index)
            console.log(apodParse.url, index);
        }
    }
}