Calling TWITCH API and displaying online/offline users of a channel

Question

I've got this working almost to where I want it. The part I can't get right may be due to the programming so I'd like to get a review.

I'm making JSON calls to the Twitch API that sends back a list of people who are streaming video on code camps channel on the Twitch website.

I have a semi populated array of users to start with. From code camps followers URL, I push some more users to the array and then I begin to build a user object and set some properties to it as well as append them to HTML. My Object is being assembled on the fly and not via a constructor nor Object.create.

To figure out if these users are streaming live or offline, I have to make another JSON call to another URL which has a stream object if user is live else a null. So I put a for loop and make a call for each of those users to see whether they are streaming or not. The problem now is, the returned JSON with the streaming information and the previous user objects are in separate scopes (though I define user globally, I can only capture the very last user object that gets built) and I can't tie the streaming property I'm getting from my second call to it. I understand the objects are being lost with each iteration of the for loop, but I've tried pushing each user as its created into an array and return the array but that's not working properly.

Can this work with the way the code is presently written or the fact that I'm making a JSON call within a JSON call is bad practice and the fault is in the structure and I should re-write it?

I need somehow to store each of those initial users that I build the HTML with and pass them to the program so I can use those guys anywhere outside.

$(function() {
  
  //JSON URLS, VARIABLES AND USER OBJECT
  var fccChannelURL = 'https://api.twitch.tv/kraken/users/freecodecamp/follows/channels/';
  var twitchChannelURL = 'https://api.twitch.tv/kraken/channels/'; //user details less streaming status
  var twitchStreamsURL = 'https://api.twitch.tv/kraken/streams/'; //stream object if user is online else null
 // var fccURL = 'https://api.twitch.tv/kraken/streams/freecodecamp?callback=?';
  var usersArr = [];
//  var streamersArr = [];
  var subs = ["ESL_SC2", "OgamingSC2", "cretetion", "freecodecamp", "storbeck", "habathcx", "RobotCaleb"];
  
  var users = {
                id: '',
                name: '',
                logo: '',
                url: '',
                stream: false
             };
  
  
  
  
  
  

  //GET FCC FOLLOWERS AND ADD THEM TO SUBS ARRAY 
  $.getJSON( fccChannelURL, function( data ) {
        for ( var i = 0; i < data.follows.length; i++ ) {
        //ADD JSON USERS TO SUBS ARRAY
        subs.push(data.follows[i].channel.display_name);  //adding ~ 8 followers
    }
 });
  


  //SET TIMEOUT TO ENSURE ARRAY IS POPULATED AFTER FIRST JSON CALL
  setTimeout(function(){

    for ( var i = 0; i < subs.length; i++ ) {
      
      $.getJSON( twitchChannelURL + subs[i], function( usersJSON ) {
      
          //POPULATE USER OBJECT 
          users.id = usersJSON._id;
          users.name = usersJSON.display_name;
          users.logo = usersJSON.logo;
          users.url = usersJSON.url;
      
      
          //HIDE LOADER
          $('.loader').hide();
        
          //CREATE HTML FOR USERS AND PREPEND TO RESULTS  
          $('#result').prepend('<div id="' + users.id + '" class="item"> <a target="_blank" href="' + users.url + '"> <img class="logo" src="'+ users.logo + '"/> <span class="name">' + users.name + '</span> </a> </div>');   
        
       }).done( function() {   
                $.getJSON( twitchStreamsURL + users.name + '?callback=?', function( streamData ){ 
                    if ( streamData.stream !== null ) {
                     console.log(streamData.stream.channel.display_name);  // gives the right guys
               
               /*****
               
               HERE I WANT TO ACCESS THE USERS OBJECTS TO SET THEIR STREAM VALUE TO TRUE. * users * is outside this functions scope  
               
               
              ********* */
               
               
                    }
              });
         });
          
      }

 }, 3000);

}); //jQuery end

EDIT:

I just ran console.dirxml() on the html element I build: #results and my for-loop is creating a div with all the users inside it on every iteration..looks like it's rendering ok but that may be a memory leak? and maybe there's another way to make the call outside of the loop?

Answer 1

Pick a style guide.

Your whitespace is truly amazingly horrific. I'm not sure why you have 7 lines of whitespace or your for loop declaration is on a different indent to your closing brace for it, but try and keep things consistent.

A lot of your code would be a lot more readable if you kept your whitespace consistent. I prefer the feross/standard style guide, but as you seem to like semicolons, flet/semistandard will do the trick for you. I've fixed your code to obey that style guide in the code dump below.

Chain promises

There's absolutely nothing wrong with using getJSON within another getJSON, but as $.getJSON returns a promise you can actually chain promises to pass your users object to the second getJSON call without having to stick it at the top of your function scope. I didn't explain that very well, but again, you should see what I mean in the code sample I've put below.

Using the chaining feature of promises helps make your code more composable and also reducing the amount of scope your variables are visible in to only what is necessary is extremely helpful for ensuring code correctness.

You can also chain promises to ensure that your getJSON call to the twitch channel URL is executed after fccChannelURL's getJSON call is completed instead of using setTimeout (which is flakey).

I've also modified your code to wait until all promises are resolved before rendering elements and removing the loader. This seems more like what you would want to do, instead of just hiding the element once you've received a single request fulfillment.

Comments describe why, not what

You have a lot of comments describing what your code is doing. You should avoid these: if you ever find yourself describing what your code is doing in comments, your code is not clear enough. Comments should ideally be a justification for why the next action is necessary, rather than what the next action is.

Security

You're creating HTML elements directly from responses to HTTP pages. While it's from Twitch so you can reasonably sure it's safe, please take more care in doing this otherwise you leave yourself wide open to XSS attacks.

Revised code:

Here's your revised code. There's a few more questions I have that I've left as notes.

$(function () {
  var fccChannelURL = 'https://api.twitch.tv/kraken/users/freecodecamp/follows/channels/';
  var twitchChannelURL = 'https://api.twitch.tv/kraken/channels/';
  var twitchStreamsURL = 'https://api.twitch.tv/kraken/streams/';

  $.getJSON(fccChannelURL).then(function (data) {
    var subs = [
      'ESL_SC2',
      'OgamingSC2',
      'cretetion',
      'freecodecamp',
      'storbeck',
      'habathcx',
      'RobotCaleb'
    ];

    for (var i = 0; i < data.follows.length; i++) {
      subs.push(data.follows[i].channel.display_name); // adding ~ 8 followers
    }
    return subs;
  }).then(function (subs) {
    var subsPromises = subs.map(function (sub) {
      return $.getJSON(twitchChannelURL + sub, function (user) {
        return {
          id: user._id,
          name: user.display_name,
          logo: user.logo,
          url: user.url
        };
      }).then(function (user) {
        // Reviewers note: I'm not sure what you are doing with this, as it doesn't seem to do anything. As a result,
        // I'm just returning the thing that was passed in. Please elaborate what you're doing here and I'll adapt the code.

        $.getJSON(twitchStreamsURL + user.name + '?callback=?', function (streamData) {
          if (streamData.stream !== null) {
            console.log(streamData.stream.channel.display_name);
            // Reviewers note: you can access "user" here to retrieve information about the user.
          }
        });

        return user;
      });
    });

    // Reviewers note: This waits for all of the subs to be resolved before then adding the elements in and 
    // hiding the loader. This seems more like what you want.
    return $.when(subsPromises).then(function (users) {
      var target = $('#result');
      users.forEach(function (user) {
        target.prepend('<div id="' + user.id + '" class="item"> <a target="_blank" href="' + user.url + '"> <img class="logo" src="' + user.logo + '"/> <span class="name">' + user.name + '</span> </a> </div>');
      });
      $('.loader').hide();
    });
  });
});

For kicks, here's your code using ES6 instead of ES5. I'm not sure if it will work 100% as I haven't tested it, but it should not be too far off. I'd recommend using ES6 if you can.

$(() => {
  const fccChannelURL = 'https://api.twitch.tv/kraken/users/freecodecamp/follows/channels';
  const twitchChannelURL = 'https://api.twitch.tv/kraken/channels';
  const twitchStreamsURL = 'https://api.twitch.tv/kraken/streams';

  $.getJSON(fccChannelURL).then((data) => {
    const subs = [
      'ESL_SC2',
      'OgamingSC2',
      'cretetion',
      'freecodecamp',
      'storbeck',
      'habathcx',
      'RobotCaleb'
    ];

    return subs.concat(data.follows.map(followee => followee.channel.display_name));
  }).then((subs) => {
    const subsPromises = subs.map((sub) => {
      return $.getJSON(`${twitchChannelURL}/${sub}`, (user) => {
        return {
          id: user._id,
          name: user.display_name,
          logo: user.logo,
          url: user.url
        };
      }).then(function (user) {
        // Reviewers note: I'm not sure what you are doing with this, as it doesn't seem to do anything. As a result,
        // I'm just returning the thing that was passed in. Please elaborate what you're doing here and I'll adapt the code.

        // Reviewers note: I assume the old callback=? parameter was ? to omit it: just remove it from the URL in that case.
        // As far as I know, the callback parameter for the Twitch API is for JSONP requests. You don't need to worry about that here.
        $.getJSON(`${twitchStreamsURL}/${encodeURIComponent(user.name)}`, (streamData) => {
          if (streamData.stream !== null) {
            console.log(streamData.stream.channel.display_name);
            // Reviewers note: you can access "user" here to retrieve information about the user.
          }
        });
        return user;
      });
    });

    // Reviewers note: This waits for all of the subs to be resolved before then adding the elements in and 
    // hiding the loader. This seems more like what you want.
    return Promise.all(subsPromises).then((users) => {
      const target = $('#result');
      users.forEach((user) => {
        target.prepend(`<div id="${user.id}" class="item">
          <a target="_blank" href="${user.url}">
            <img class="logo" src="${user.logo}"/>
            <span class="name">${user.name}</span>
          </a>
        </div>`);
      });
      $('.loader').hide();
    });
  });
});