0

I've written a few lines of code to tackle the following problem:

Get the TwitchTV UserID based on the username. The usernames are in an arrray. After hitting a button a GET AJAX request will be called to GET the UserID and to push it in another array and the call to the API will made via AJAX after hitting a button.

My problem is that if I hit the button again the usersID are in wrong order.

If I set async: false, it works.

Is this problem because of Asynchronous AJAX? What would be the approach for a valid fix? Use callback? A hint in the right direction would be appreciated.

The comments in the code are for testing.

Code:

<script>

        var users = ["ESL_SC2", "OgamingSC2", "cretetion", "freecodecamp", "storbeck", "habathcx", "RobotCaleb", "spicyjewlive"];
            clientID = "?client_id=XXX";
            userID = [];
            userStatus = [];
for(var i=0; i<users.length; i++){
          idCall(users[i]);

        };
function idCall (data){
            $.ajax({
              type: "GET",
              url: "https://api.twitch.tv/kraken/users/" + data + clientID,
              async: false,
              cache: false,
              dataType: "jsonp",
              success: function (data){
                console.log(data._id);
              }, 
              error: function (data) {
                console.log("error");
              }});
        };
</script>
Liam
  • 27,717
  • 28
  • 128
  • 190
Mario
  • 870
  • 9
  • 20
  • It's difficult to tell where one function ends and another begins in this code. Perhaps you just have a curly-brace typo somewhere which is confusing the intended logic? Can you simplify the overall example to just the specific problem occurring? – David Jul 05 '17 at 12:33
  • Why do you put the scripts after the `

    `? Also, please check your console for errors...are there any? Your code has some syntax errors...missing `}` for example.

    – Ionut Necula Jul 05 '17 at 12:33
  • @David: code updated. – Mario Jul 05 '17 at 12:35
  • 2
    Yes, it is because ajax is asynchronous. You're doing a for loop and assuming that the results will also come out in that order, but because one ajax request might complete before the other, the order may differ every time you execute the function. I've actually answered a similar question earlier today: https://stackoverflow.com/questions/44919581/jquery-waiting-for-ajax-to-complete-within-each-iteration – vi5ion Jul 05 '17 at 12:35
  • 3
    @Mario: Well, you definitely *don't* want to use `async: false`. While it may "work" in any given case, it's not officially supported these days and is a bad habit to get into overall. If the order of the results matters, then the two approaches I can think of would be either (1) perform the AJAX calls in serial instead of parallel, having each one be invoked by the completion of the previous one, or (2) sort the results each time a new result is added to the collection. – David Jul 05 '17 at 12:40
  • @vi5ion: thanks, so this issue is because of the nature of ajax. What would be a valid solution within ES6, workaround with callback? – Mario Jul 05 '17 at 12:41
  • @David: thanks, yep, have opened the console and seen the depreciation warning about async:false. So the best way would be to make loop after loop? so for array length 8, 8 ajax calls depending on each other? – Mario Jul 05 '17 at 12:44
  • That is indeed what he's suggesting and also why I linked to that other post. Because doing that 8 times is a bit redundant. You can solve that with a recursive function. – vi5ion Jul 05 '17 at 12:45
  • @Mario: It's *a* way, whether or not it's the *best* way is entirely subjective. But it would at least preserve the ordering of the results, since each subsequent request wouldn't be sent until the previous response is received. You might structure it recursively, or perhaps keep the loop structure and have the loop body simply append each call as a response to the promise of the first call. So the loop would finish immediately, and the result of the loop is a chain of promise objects. – David Jul 05 '17 at 12:46
  • @David: thanks, I will give it a try. But I have a feeling that there must be an easier solution for this, just imagine I'd have 100 users. – Mario Jul 05 '17 at 13:05
  • @Mario: How does the number of users make a difference? You either perform the operations in serial (which is a pretty simple loop, appending calls to a promise object) or you perform the operations in parallel and sort the results each time an operation completes. Neither approach is particularly complex, and the complexity doesn't change with the number of elements in the array. – David Jul 05 '17 at 13:11
  • @David: I thought your suggestion is to hardcode the ajax loops in each other, that would make a difference if I've 10 or 100 users. – Mario Jul 05 '17 at 13:32
  • 1
    @Mario: No, I wouldn't suggest doing that at all. A loop building a chain of promises would likely be ideal for the serial approach, though a more recursive-like structure of callback functions would work just as well. (For example, each callback would check if it's the last one in the series and, if not, invoke the next one.) 100 operations in serial might take longer than desired, so parallel operations might become more useful there. In which case you'd just have to sort your results with each response. (Though with 100+ operations, rate-throttling might become important too.) – David Jul 05 '17 at 13:35

3 Answers3

1

Use an array of request promises and update the dom after all those are resolved. The results will be in same order as original users array

var requestPromises = users.map(idCall);

$.when.apply(null, requestPromises).then(function(){
   var dataArray = [].slice.call(arguments);   
   // now loop over data array which is in same order as users
   dataArray.forEach(function(userData){
     // do something with each userData object
   });   
}).fail(function(){
   console.log("Something went wrong in at least one request");
});

function idCall(user) {
  // return the ajax promise
  return $.ajax({    
    url: "https://api.twitch.tv/kraken/users/" + user + clientID,
    dataType: "jsonp"
  }).then(function(data) {
    // return the data resolved by promise
    return data;
  });
};
charlietfl
  • 170,828
  • 13
  • 121
  • 150
  • thanks. I think your answer is a good example of me not understanding js. I will give the concept of promises a look. any simpler solution with "let" or ...? – Mario Jul 05 '17 at 13:03
  • Not if you want them in order. There is no guarantee how long each request takes and therefore, as you are seeing, the order that the data is returned – charlietfl Jul 05 '17 at 13:05
  • I have published another solution above – Virk Bilal Jul 05 '17 at 13:13
  • thank you @charlietfl. I'd time to test it now, and your solution works very good. The concept of promises is still not fully understood, but a little bit more. Also .map() is a helpful command. – Mario Jul 07 '17 at 11:05
0

What would be the approach for a valid fix?

I think @charlieftl answer is a good one. I don't think I can really improve on this. I've mainly added this to try and explain your difficulties here. As I mentioned in a previous question you posted, you need to read and understand How do I return the response from an asynchronous call?.

If you had control of the server side then a simpler option is to send the array server side in order and use this to maintain the order, but your using a third party API so this isn't really practical.

A simpler option would be to use a callback method:

 var users = ["ESL_SC2", "OgamingSC2", "cretetion", "freecodecamp", "storbeck", "habathcx", "RobotCaleb", "spicyjewlive"];
 var clientID = "?client_id=XXX";
 var userID = [];
 var userStatus = [];
 idCall(users);


 function idCall (users, x=0){
        if (x < users.length)
        {
            var data = users[x];
            $.ajax({
              type: "GET",
              url: "https://api.twitch.tv/kraken/users/" + data+ clientID,
              cache: false,
              dataType: "jsonp",
              success: function (data){
                console.log(data._id);
              }, 
              error: function (data) {
                console.log("error");
              }})
              .then(function(){
                  idCall(users, x++);
              });
          }
    };

though like I said, charlies answer is better. This will guarantee the order and won't lock the UI. But it's slow.

Is this problem because of Asynchronous AJAX?

Yes, well asynchronous processing on a server and ordering.

When you first this open up the network panel of your debugging tool and watch the HTTP requests. What you'll see is a load of requests getting sent off at the same time but returning in different orders. You cannot guarantee the response order of a async request. It depends on the server response time, load, etc.

Example: enter image description here

These request we're sent in order A,B,C,D,E,F,G but the responses we're received in B,C,E,F,G,D,A this is the nub of your problem.

Setting async to false does this:

enter image description here

Now no request will be sent until the previous has returned but. So stuff is sent A-G and returns A-G but this is A LOT slower than the previous example (the graphs here aren't to scale, I struggled to capture a good async:false example as I never use it, see reasons below).

Never do this (pretty much ever). What will happen is the UI thread will become locked (Javascript/browsers are single threaded, mostly) waiting for an external call and your web site will become unresponsive for long periods of time. Locking this thread doesn't just stop Javascript, it stops the entire site, so links don't work, hover overs, everything, until the async call returns the thread back to the UI.

Community
  • 1
  • 1
Liam
  • 27,717
  • 28
  • 128
  • 190
  • thank you Liam, I really appreciate your help and especially the link for the async call (very good summary). The concept of ajax and promises is understood. With (.done, .fail,...) the mapping and the rest as told above, well, this needs more time. thank you. – Mario Jul 07 '17 at 11:08
-1

Yes, it is because of the AJAX calls. One AJAX call might finish before the other and therefore the different order.

An obvious solution is to use jQuery promises but another solution to this problem could be that you sort your array after your AJAX calls have been completed.

You can sort your array according to the username string like this:

data.sort();

If you have an array of objects, which you don't in the example code, you can sort it like this:

data.sort(function (a, b) { return (a.Name > b.Name) ? 1 : ((b.Name > a.Name) ? -1 : 0); } );

After going through the comments, I thought to give some more code that can a better idea on how to solve this.

<script>

var users = ["ESL_SC2", "OgamingSC2", "cretetion", "freecodecamp", "storbeck", "habathcx", "RobotCaleb", "spicyjewlive"];
clientID = "?client_id=XXX";
userID = [];
userStatus = [];

var count = 0;
for(var i=0; i<users.length; i++){
    idCall(users[i]);
}

function idCall (data){
   $.ajax({
        type: "GET",
        url: "https://api.twitch.tv/kraken/users/" + data + clientID,
        async: false,
        cache: false,
        dataType: "jsonp",
        success: function (data){
            console.log(data._id);
            userID.push(data._id);
            count++;
            if( count === users.length - 1 ) {
                userID.sort();
            }
        }, 
        error: function (data) {
            console.log("error");
        }

   });
}

Virk Bilal
  • 640
  • 1
  • 5
  • 10
  • thanks Virk. But how can I sort an array, when I push the data in the array not in order, because of the async ajax call. i would have no connection to username and userID. – Mario Jul 05 '17 at 12:55
  • You should do this sort after the for loop that you have in your code. You can create a boolean that would keep checking if the user.length has reached it's maximum, and if it has then fire the sort function on the array – Virk Bilal Jul 05 '17 at 13:01
  • I have edited my answer to give you a better understanding. It is not using a boolean but an int 'count' instead. Though it is possible to do it with a flag too. – Virk Bilal Jul 05 '17 at 13:12
  • 1
    Sorting wrong variable in update. Also never ever use `async: false` it is deprecated and a terrible practice – charlietfl Jul 05 '17 at 13:15
  • To do what you are thinking would need to push each response into an array... then sort that array on last count and consume the array then – charlietfl Jul 05 '17 at 13:17
  • @VirkBilal. yes, with the sort() I'd get always the same result in the array, just i don't which userID belongs to which Users, since the userID is sorted, for example userID[2] = users[0] and so on... This sorting seems not to be an appropriate solution. But I think the sorting approach could be. – Mario Jul 05 '17 at 15:32
  • You should probably save your data as an array of objects which would help you recognise which userID belongs to which user. – Virk Bilal Jul 05 '17 at 19:26