0

So i wrote this piece of code in javascript :

// Additional initialization code such as adding Event Listeners goes here
            FB.api('593959083958735/albums', function(response) {
                if(!response || response.error) {
                    // render error
                    alert("Noo!!");
                } else {
                    // render photos
                    for(i=0; i<response.data.length; i++){
                        var albumName = response.data[i].name;
                        var albumCover = response.data[i].cover_photo;
                        var albumId = response.data[i].id;
                        console.log(albumName);
                        FB.api( albumCover, function(response) {
                            if(!response || response.error) {
                                // render error
                                alert("Noo!!");
                            } else {
                               // render photos
                               $("ul").append('<li>'+
                                    '<a href="testFile.HTML" data-transition="slidedown">'+
                                    '<img src= "' + response.picture + '"  />'+
                                     '<h2>' + albumName + '</h2>'+
                                     '<p> Test Paragraph</p>'+
                                     '</a>'+
                                     '</li>')
                                .listview('refresh');
                            }
                        });                                 
                    }
                }
            });

I am using facebooks javascript API , to import the photo albums of a facebook page to my jquery mobile page and put them in a listView. As you see i create the listView dynamically. The listView has as thumbnail the albums coverPhoto and headline , albums name.

However something is horribly wrong here. The result of the code above can be seen here

The thumbnails are all correct but the album names are wrong. In every cell i get the last album name. When i console.log(albumName) in my code as you see , i get all the names correct. But inside the second call to FB.api the "albumName" variable holds only the last album name.

Any idea what is going on here? Its literally driving me nuts...

Johny Jaz
  • 875
  • 2
  • 13
  • 26

2 Answers2

2

Looks like you have the famous i for loop problem where you only have a reference to i.

                for (var i = 0; i < response.data.length; i++) {  //added the var here
                    (function (i) {  //created a function
                        var albumName = response.data[i].name;
                        var albumCover = response.data[i].cover_photo;
                        var albumId = response.data[i].id;
                        console.log(albumName);
                        FB.api(albumCover, function (response) {
                            if (!response || response.error) {
                                // render error
                                alert("Noo!!");
                            } else {
                                // render photos
                                $("ul").append('<li>' +
                                    '<a href="testFile.HTML" data-transition="slidedown">' +
                                    '<img src= "' + response.picture + '"  />' +
                                    '<h2>' + albumName + '</h2>' +
                                    '<p> Test Paragraph</p>' +
                                    '</a>' +
                                    '</li>')
                                    .listview('refresh');
                            }
                        });
                    })(i);  //call the function with i
                }
epascarello
  • 204,599
  • 20
  • 195
  • 236
2

It's actually not the i that's the problem, but rather albumName.

Most JavaScript engines are very forgiving and allow you to declare the same variable with the var keyword multiple times. That's what you're doing here. It's important to note that the for keyword does not introduce a new scope; so when you (re-)declare each variable within the loop (albumName, albumCover, and albumId) you're overwriting the value previously stored in the same variable.

This comes back to bite you because the callback you pass to FB.api closes over these local variables and (clearly) executes asynchronously. By the time the callback is actually run, the for loop has completed and these variables are all set to their last value.

The exception, of course, is albumCover, as you already observed (you said the thumbnails were all correct). That's because you pass this value to FB.api synchronously; i.e., you pass it within the loop, so it isn't closed over.

For a better understanding of closures in JavaScript, see How do JavaScript closures work?

To be clear: epascarello's answer should solve the problem for you. Wrapping up your logic in an anonymous function does introduce a new scope (a useful trick that can be used in a variety of scenarios), and so the value of albumName, etc. getting closed over is different for each callback. I just wanted to point out that it wasn't the i per se causing the problem (since your callback doesn't close over i at all).

Community
  • 1
  • 1
Dan Tao
  • 125,917
  • 54
  • 300
  • 447
  • wow.. What an amazing answer you gave there ! I completely understand now why it was not working. One small question though. I saw that you talked about re-declaring the same variable using the keyword var. But even if i declare them with this keyword outside the loop it still doesnt work. To my understanding , the problem was the asychronous call , which as you said , by the time is called , the for loop is already done and so the variables are set to their last values right? – Johny Jaz Jun 21 '13 at 09:00
  • @JohnyJaz: Yes, that's right. My point about "re-declaring" the variables is that it doesn't matter whether you do so inside or outside the loop. Either way, it's the same variable being closed over in your callback on every iteration. Basically, `var i = 1; var i = 2;` (looks like two declarations) is the same thing as `var i = 1; i = 2;` (a declaration, then an assignment). You needed a new scope to solve this problem so that what *looked* like new variables being declared actually *was*. Make sense? – Dan Tao Jun 21 '13 at 14:35
  • Yes makes perfect sense :) . Thank you a lot. – Johny Jaz Jun 24 '13 at 07:44