1

I can't for the life of me select this correctly? I may have the for loop wrong so please advice if that is the case:

    function(data, callback){
        data.champNames = {};
        for(var c in data.champId){
            val = data.champId[c];
            var surl = 'https://global.api.pvp.net/api/lol/static-data/euw/v1.2/champion/'+ [val] + '?api_key=' + api_key;
            request(surl, function(err, response, body){
                if(!err && response.statusCode == 200){
                    var json = JSON.parse(body);                        
                    //data.champNames.push(json[val].name);
                    console.log(json);                        
                } else {
                    console.log('Error in Champ Name');
                }
            })
            console.log(val);                
        } console.log(data);
    }

The data.champNames.push(json[val].name) will not work, JSON is returned as the below in console:

{ id: 1, key: 'Annie', name: 'Annie', title: 'the Dark Child' }
{ id: 76,
  key: 'Nidalee',
  name: 'Nidalee',
  title: 'the Bestial Huntress' }
{ id: 15,
  key: 'Sivir',
  name: 'Sivir',
  title: 'the Battle Mistress' }
{ id: 103,
  key: 'Ahri',
  name: 'Ahri',
  title: 'the Nine-Tailed Fox' }

Data is a global variable used within an async waterfall.

ChampId is already in an array and is returned as:

  champId: [ 22, 236, 76, 21, 36, 133, 24, 103, 81, 79, 45, 15, 1, 0 ],
Mihai Alexandru-Ionut
  • 47,092
  • 13
  • 101
  • 128
  • 1
    The issue has nothing to do with JSON. Note that the code won't work as you expect anyway. The `for` loop is synchronous, so `console.log(data)` will be executed **before** any of the `request` callbacks is executed. You should read [Why is my variable unaltered after I modify it inside of a function? - Asynchronous code reference](http://stackoverflow.com/q/23667086/218196) – Felix Kling Dec 12 '16 at 15:01
  • Before `console.log(val);` you are missing `;`. – Legionar Dec 12 '16 at 15:04
  • @Legionar: `;` are optional. – Felix Kling Dec 12 '16 at 15:05
  • Hey, would you mind sharing how I would change this to do as I want? I want to be able to display these on the page, along with howmany times they've played the champ. I've got the number of times already stored, I just need to now get the names. – Alex Wilkes Dec 12 '16 at 15:06
  • 1
    You already gave your function a `callback` parameter, seemingly knowing of its asynchrony. So then why don't you use it?! – Bergi Dec 12 '16 at 15:08
  • Hello, I may be trying to run before I can walk but I'm learning this as I go. What do you mean why I don't I use it? – Alex Wilkes Dec 12 '16 at 15:12
  • *"What do you mean why I don't I use it?"* The function accepts a parameter `callback`. Why did you add it and why are you not referencing it inside the function? – Felix Kling Dec 12 '16 at 15:14
  • Where abouts would I reference callback? As I said this is what I've written from finding bits and pieces doing similar things to what I wanted. I don't particularly know what callback even does. Sorry! – Alex Wilkes Dec 12 '16 at 15:18
  • Objects do not have a push method... – epascarello Dec 12 '16 at 15:18
  • Well, as you can see, copying and pasting pieces without really understanding them won't get you far. Btw, to access the name of the object from the response, you just need to do `json.name`. However, that is the least of your problems. – Felix Kling Dec 12 '16 at 15:31
  • Who said it was copied and pasted? Non of it is, I've tried to fully understand what I can, got stuck, and came here. Thanks anyway. – Alex Wilkes Dec 12 '16 at 15:53

3 Answers3

1

data.champNames={} is a dictionary in your case.To use push method you need to use an array. Try this:

data.champNames = [];

Or,if you want use dictionary use this:

data.champNames["key"]=value;

Updated answer:

One method is to use Immediately-Invoked Function Expression

for(var c in data.champId){
        val = data.champId[c];
        var surl = 'https://global.api.pvp.net/api/lol/static-data/euw/v1.2/champion/'+ [val] + '?api_key=' + api_key;
        (function(val){
            request(surl, function(err, response, body){
            if(!err && response.statusCode == 200){
                var json = JSON.parse(body);                        
                //data.champNames.push(json[val].name);
                console.log(json);                        
            } else {
                console.log('Error in Champ Name');
            }
        })(val);
        console.log(val);                
 } 

Another solution is to use let keyword.

ES6 provides the let keyword for this exact circumstance. Instead of using closures, we can just use let to set a loop scope variable.

Please try this:

for(let c in data.champId){
        let val = data.champId[c];
        var surl = 'https://global.api.pvp.net/api/lol/static-data/euw/v1.2/champion/'+ [val] + '?api_key=' + api_key;
        request(surl, function(err, response, body){
            if(!err && response.statusCode == 200){
                var json = JSON.parse(body);                        
                //data.champNames.push(json[val].name);
                console.log(json);                        
            } else {
                console.log('Error in Champ Name');
            }
        })               
    }
Mihai Alexandru-Ionut
  • 47,092
  • 13
  • 101
  • 128
  • Hey, thanks. I changed it to an array and used: data.champNames.push(json[c].name); however I get cannot read property of name "Undefined" – Alex Wilkes Dec 12 '16 at 15:06
  • 2
    @InchHigh: This cannot work because `c` doesn't have the value you expect (`val` wouldn't either btw). See [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/q/750486/218196). You need another way to iterate and process your data asynchronously. You should look into [promises](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise). – Felix Kling Dec 12 '16 at 15:10
  • How many objects do you have in json array ? For example, in your `champId` , first element is 22, and in your example JSON have only 4 elements. – Mihai Alexandru-Ionut Dec 12 '16 at 15:14
  • @FelixKling,yes, you're right..he needs to use closures or `.bind` function. – Mihai Alexandru-Ionut Dec 12 '16 at 15:16
1

While accessing the object seems to be an issue, the bigger issue is actually with how you process the data. You are performing an asynchronous operation (request(...)) inside a synchronous loop (for...in). Because of the way JavaScript works, that means the loop will finish first before any of the request callbacks is executed.

The issues in your code have been covered extensively in other questions and resources, so I highly recommend to read the following before you do anything else:

In order to solve your particular problem, you should look into Promises. We will create a new promise for each entry in the data.champId array. A promise is basically a placeholder for a future value. A promise can either resolve to a value or be rejected if there was an error. Here, each promise will be a placeholder for the response received via request, which is an asynchronous process. In particular, each of these promises will resolve to the name of the response object. Helper methods such as Promise.all allow us to perform an action once multiple promises are resolved.

function fetchNames(data) {
  // Promise.all returns a new promise that resolves when all promises passed to it
  // are resolved
  return Promise.all(data.champId.map(function(id) {

    var surl = 'https://global.api.pvp.net/api/lol/static-data/euw/v1.2/champion/'+ id + '?api_key=' + api_key;

    // Create a new promise for each entry in data.champId
    return new Promise(function(resolve, reject) {

      request(surl, function(err, response, body){
        if (!err && response.statusCode == 200){
          var result = JSON.parse(body);
          resolve(result.name); // the response is an object with a property name                    
        } else {
          reject('Unable to load entry "' + id + '"');
        }

    });
  });

}

fetchNames({champId: [ 22, 236, 76, 21, 36, 133, 24, 103, 81, 79, 45, 15, 1, 0 ]})
  .then(function(names) {
    console.log(names);
  })
  .catch(function(error) {
    // An error occured
  });
Community
  • 1
  • 1
Felix Kling
  • 795,719
  • 175
  • 1,089
  • 1,143
0

you are trying to do a push in an object: you declared data.champNames = {}, but for a push you need an array data.champNames = [];).

Reading the code I can assume you're trying to read the val-th element of the JSON array, not the JSON element with id == val. If you want to get the name of the element with id==val you have to do a search inside the JSON object.

Ketty D.S.
  • 19
  • 4