Search code examples
javascriptjqueryloopsechonest

Infinite loop in $(document).ready when interfacing with API


I'm using CodePen on this project, which is how I discovered that there is an infinite loop in my code. As an aside - it estimates the line number of the infinite loop to be line 0, which is why I suspect it has something to do with the $(document).ready() piece of code.

I'm a jQuery / API beginner, so apologies if this is obvious.

Here is the code:

$(document).ready(function() {
  var key = '*****************';
  var i = 0;
  var hotArray = [];

  function isInArray(value, array) {
    return array.indexOf(value) > -1;
  }

  function getMusic() {
    $.getJSON('http://developer.echonest.com/api/v4/song/search?api_key=' + key + '&artist=led+zeppelin&sort=song_hotttnesss-desc&results=50&bucket=song_hotttnesss', function(data) {
      while ($('.songs li').length < 10) {
        if (!(isInArray(data.response['songs'][i]['hotttnesss'], hotArray))) {
          $('.songs').append('<li>' + data.response['songs'][i]['title'] + '</li>');
          hotArray.push(data.response['songs'][i]['hotttnesss'])
        }
      }
      i++;
    });
  };

  $('.click').click(getMusic);
});

Brief explanation of the code:

I'm trying to get the top 10 songs by Led Zeppelin when I click on a certain bit of the page (later to be expanded to user-entered artist). But the way the Echo Nest API works, many, many duplicates are returned. For example, "Stairway to Heaven" and "Stairway To Heaven" are considered two separate songs.

To get around this problem, I'm using the "hotttnesss" value of each song, which is unique for every song. The only way two songs by the same artist can have the same hotttnesss is if they are actually the same song, as is the case with these duplicates.

So, I'm stepping through each of the returned songs while the number of reported songs is under 10, and reporting the song if its hotttnesss value is not in the array of the already-reported-songs' hotttnesss values. After reporting the song, the song's hotttnesss value is added to the array.


Solution

  • The i++; should be inside the while loop, not after it. You also need to set i to 0 in your ajax callback, and limit that same loop to i < data.response['songs'].length.

    E.g.:

    function getMusic() {
        $.getJSON('http://developer.echonest.com/api/v4/song/search?api_key=' + key + '&artist=led+zeppelin&sort=song_hotttnesss-desc&results=50&bucket=song_hotttnesss', function(data) {
            var i, songs = data.response['songs'];
            for (i = 0; $('.songs li').length < 10 && i < songs.length; ++i) {
                if (!(isInArray(songs[i]['hotttnesss'], hotArray))) {
                    $('.songs').append('<li>' + songs[i]['title'] + '</li>');
                    hotArray.push(songs[i]['hotttnesss'])
                }
            }
        });
    }
    

    or I'd just use Array#forEach (you can shim it if you need to support IE8):

    function getMusic() {
        $.getJSON('http://developer.echonest.com/api/v4/song/search?api_key=' + key + '&artist=led+zeppelin&sort=song_hotttnesss-desc&results=50&bucket=song_hotttnesss', function(data) {
            data.response['songs'].forEach(function(song) {
                if (!(isInArray(song['hotttnesss'], hotArray))) {
                    $('.songs').append('<li>' + song['title'] + '</li>');
                    hotArray.push(song['hotttnesss'])
                }
            });
        });
    }
    

    Side notes:

    1. Function declarations (such as your getMusic) are not statements, you don't need a statement terminator (;) after them. (You do after a function expression that's, say, part of an assignment.)

    2. song['hotttnesss'] can be written song.hotttness, response['songs'] as response.songs, etc. Unless the property name starts with a digit or contains a character that isn't valid in JavaScript identifier names, there's no need for quoting it.

    3. There's no purpose served by putting () around a function call in most cases, so

      if (!(isInArray(song['hotttnesss'], hotArray))) {
      

      could more clearly be

      if (!isInArray(song['hotttnesss'], hotArray)) {
      

      (The only times when there's a purpose to them involve inline-invoked function expressions.)

    4. You don't need your isInArray function; jQuery already has one: $.inArray. (It's well worth your time to read through the jQuery API beginning to end. Literally only takes an hour or two, and you'll find all sorts of useful things you may not know about.)