Jquery Defer & Each Loop

Question

I'm a bit new to JSON and Deferrers, so apologies it the answer is obvious.

I am working with the pokeapi.co and I am trying to pull out details of various pokemon using getJSON. I created a function which takes an array of URLs, runs a getJSON function to pull the data from the URLs, iterate over the data and push it into an array. I am trying to use the defer object and promises to determine when all that has been done so I can use the data in another function. Unfortunately, I seem to be doing something wrong.

var url = [
  "http://pokeapi.co/api/v2/pokemon/1/",
  "http://pokeapi.co/api/v2/pokemon/2/"
];

function getPokemonDetails(url){
  var def = $.Deferred();
  var promises = [];
  var pokemon;
  $.each(url, function(i, index){
    var deferred = $.Deferred();
    $.getJSON(index, function(data){
      pokemon = [data.name, data.stats];
      var abilityURLs = [];
      $.each(data.abilities, function(a, abilities){
        abilityURLs.push(abilities.ability.url)
      });

      pokemon.push(abilityURLs);
      deferred.resolve(pokemon);
    }); //End getJSON

    deferred.done(function(data){
      promises.push(data);
      console.log(promises);
    })// End Deferred
  }); //End Each

  $.when(...promises).done(function(){
    console.log(arguments.length);
  });
}

getPokemonDetails(url);

At the moment, when the console.log runs, I am expecting the length of the promises array to be 2, but at the moment it returns 0. I am not sure exactly where I went wrong.

When creating an array of promises, .map is usually the right tool. You need the array of promises immediately, which is part of your problem; you aren't pushing to the array until the promise is done, which is incorrect. — Kevin B, Jun 26 '17 at 16:00
and when using `$.getJSON` it's usually an anti-pattern to create your own jQuery deferred objects around them — Alnitak, Jun 26 '17 at 16:01
Additionally, you don't need `$.Deferred`, $.getJSON already returns a promise. — Kevin B, Jun 26 '17 at 16:01
Avoid the [deferred antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, Jun 26 '17 at 21:55

Alnitak · Accepted Answer · 2017-06-29T13:36:53.077

1

Firstly, instead of using $.each and a push, since you have a 1:1 mapping from source array to destination array, you should use Array.prototype.map.

Secondly, since $.getJSON already returns a $.Deferred() there's absolutely no need to create one of your own.

So, if I follow your current code correctly, it can be replaced completely with this:

function getPokemonDetails(url_list) {
    return $.when(... url_list.map(url =>
        $.getJSON(url).then(
            data => [ data.name, data.stats, [
                data.abilities.map(ability => ability.ability.url)
            ]]
        )
    ));
}

The inner .then call takes the data returned from each AJAX call and returns a promise that resolves to the array of name, stats and nested array of abilities.

For what it's worth, I would probably however split this into two functions - one that handles a single URL, and another that can handle multiple ones:

const getPokemonDetails = url => $.getJSON(url).then(
    data => [ data.name, data.stats, [
                data.abilities.map(ability => ability.ability.url)
            ]]);

const getMultiPokemonDetails = list => $.when(...list.map(getPokemonDetails));

edited Jun 29 '17 at 13:36

answered Jun 26 '17 at 16:12

Alnitak

334,560
70
407
495

HI Alnitak, thanks for your reply. the three dots before the url_list in your solution, is that an array spread operator? I thought they had to be right next to the array? – tcherokee Jun 26 '17 at 18:47
@tcherokee Yes, it's an array spread operator, and the white space is legal – Alnitak Jun 26 '17 at 21:44
Hi Alnitak, apologies for not replying for a while, I've been doing some research and reading to fully understand your solution. It uses a few ideas I hadn't gotten around to learning yet (arrow functions for example). So I have a few questions. one, for some reason, the solution above only shows one result, even though two URLs get passed into the url_list. I am not sure why. Second question, if I expand the above function (i.e. `url_list.map(function(){...})` as opposed to `url_list.map(url=>...`, it stops working. I was hoping you would know why? – tcherokee Jun 29 '17 at 11:44
@tcherokee the returned promise _should_ give two results `getPokemonDetails(list).then(function(result1, result2) { ... }`. ( `$.when` doesn't generate an array). As for the arrow functions, don't forget that a single-expression arrow function has an implicit `return`, so if not using that syntax you must _explicitly_ `return` the desired result. – Alnitak Jun 29 '17 at 13:24
I think I have everything as it should be. I added all the working data into a pen so you could take a look. https://codepen.io/tcherokee/pen/jwZPej – tcherokee Jun 29 '17 at 15:07
@tcherokee you're getting there, but I see a few minor issues, like for example where you end up storing a promise in the fourth element of the array instead of the return values from that promise. In `getPokemonAbility` you `.map` over multiple values, but you don't "spread" those array values into `$.when`. Consider using `Promise.all([ array ])` instead of `$.when(p1, p2, ...)` – Alnitak Jun 29 '17 at 16:49
also the final data you write to the console (since you're using `$.when`) should be all the function parameters (accessed via `arguments`) and not just the single `data` parameter. This is again somewhere where `Promise.all` might help. – Alnitak Jun 29 '17 at 16:50
Thanks for all your help Alnitak, I've gotten it working now and I've updated the codepen with the working code. Let me know what you think of my code. – tcherokee Jun 30 '17 at 10:39
it's getting better - there's no need for the `Promise.resolve()` call, nor the outer `$.when()` at the bottom. – Alnitak Jun 30 '17 at 10:51

Jquery Defer & Each Loop

1 Answers1