1

I suspect I've fundementally misunderstood Javascript promises, any ideas?

I have a pretty function that queries a database containing music that looks like this:

function searchDatabaseForTrack(query,loadedResults){
    loadedResults = loadedResults || [];
    desiredResults = 100;
    if (loadedResults.length < desiredResults) {
        try {
            databaseApi.searchTracks(query, {"offset":loadedResults.length, "limit":"50", }).then(function(data){
                i=0
                if (data.tracks.items.length == 0) {
                    console.log(`Already loaded all ${loadedResults.length} tracks!`)
                    console.log(loadedResults)
                    return loadedResults;
                }
                else {
                    for (thing in data.tracks.items){
                        loadedResults.push(data.tracks.items[i]);
                        i=i+1;
                    }
                    console.log(loadedResults.length, " tracks collected");
                    searchDatabaseForTrack(query,loadedResults)
                }
                });
        } catch(err) {
            console.log("ERROR!", err)
            console.log(loadedResults)
            return loadedResults;
        }
    } else {
        console.log(loadedResults)
        return loadedResults;
    }
}

And then a bit later, I try to call and use the data retrieved.

 function getArtistTracks(artistName){
    searchDatabaseForTrack(artistName).then(function(data){
        console.log(songs);
        songs.sort(function(a,b){
            var c = new Date(a.track.album.release_date);
            var d = new Date(b.track.album.release_date);
            return d-c;
        });
        console.log("songs", songs);
        var newsongs=[];
        i=0
        for (song in songs) {
            newsongs.push(songs[i].track.uri);
            i++
        };
        return newsongs;
    });
}

What I'm trying to do is get the second function "getArtistTracks" to wait for the completion of the query in the first function. Now I could just call the databaseApi.searchTracks directly, but there's a limit of 50 tracks returned per result — which kind of screws me over.

omgitsbxrt
  • 21
  • 3

2 Answers2

0

searchDatabaseForTrack().then(...) shouldn't work since searchDatabaseForTrack() doesn't return a promise, so you can either return a promise or use an async function.

instead of a recursive function, you could simply call databaseApi in a for loop,

the desiredResult should be an argument and not hardcoded in the function,

async function searchDatabaseForTrack(query, desiredResults){
    let loadedResults = [], data, currOffset = 0;
    const iterations = Math.ceil(desiredResults / 50);

    for(let n = 0 ; n < iterations; n++){
        cuurOffset = n * 50;
        data = await databaseApi.searchTracks(query, {"offset":currOffset, "limit":"50", });

        if (data.tracks.items.length == 0) {
            console.log(`Already loaded all ${loadedResults.length} tracks!`)
            console.log(loadedResults)
            return loadedResults;
        }
        else {
            loadedResults = loadedResults.concat(data.tracks.items);
            console.log(loadedResults.length, " tracks collected");
        }
    }

    return loadedResults;
}

the rest should be fine as long as you add .catch() to handle errors ( as mentionned in previous answer ) which are thrown automatically without the need of the try/catch block :

function getArtistTracks(artistName, 100){
    searchDatabaseForTrack(artistName).then((songs) => {
      // your previous code
    })
    .catch((err) => {
      // handle error
    });
});
Taki
  • 17,320
  • 4
  • 26
  • 47
  • Remember that [`Array.prototype.concat()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat) returns a new array - the original array(s) remain unchanged. – Roamer-1888 Jul 03 '18 at 23:35
  • 1
    @Roamer-1888 i originally did `.forEach()` and `.push()` the values to the `loadedResults` then chaged it quickly to `.concat()` , thnx for pointing that out. – Taki Jul 03 '18 at 23:39
  • Also noticed a typo (cuur instead of curr) etc, but this is a brilliant solution! Took me a bit to break down and understand the code, but thanks! Now I have a whole new understanding of promises :) – omgitsbxrt Jul 10 '18 at 09:41
-1

Have searchDatabaseForTrack use Promise.all to return the loadedResults after all results have been gotten. Also, make sure not to implicitly create global variables as you're doing with thing. For example, try something like this:

async function searchDatabaseForTrack(query) {
  const desiredResults = 100;
  const trackPromises = Array.from(
    ({ length: Math.ceil(desiredResults / 50) }),
    (_, i) => {
      const offset = i * 50;
      return databaseApi.searchTracks(query, { offset, limit: 50 });
    }
  );
  const itemChunks = await Promise.all(trackPromises);
  const loadedResults = itemChunks.reduce((a, { tracks: { items }}) => (
    [...a, ...items]
  ), []);
  return loadedResults;
};

and

searchDatabaseForTrack(artistName).then((loadedResults) => {
  // do stuff with loadedResults
})
.catch((err) => {
  console.log("ERROR!", err)
  // handle error
});
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
  • @Downvoter, what did I do wrong? I would appreciate an explanation so I can fix it – CertainPerformance Jul 03 '18 at 21:08
  • 1
    I didn't downvote, but this is really, really confusing code to follow. – jfriend00 Jul 03 '18 at 21:18
  • Is it the use of `Array.from` to construct the array of promises? I've been under the impression that `Array.from` was the preferred functional way to create an array from scratch (more understandable than an asynchronous function that calls itself recursively and needs to return all results to the initial caller, at least). I could have used a `for` loop instead, but [loops are evil](https://www.quora.com/Can-we-say-that-the-for-loop-will-be-the-new-goto-given-the-increasing-popularity-of-functional-programming/answer/Tikhon-Jelvis?srid=3SJB&share=1), as many say – CertainPerformance Jul 03 '18 at 22:51
  • 1
    It takes a fairly advanced programmer a fair amount of time studying this code to figure out what it does and how it works. IMO, that's not great code. Coding is not a brevity contest. It's a contest to write correct, clear, easy to understand and maintainable code. Sometimes a loop is the best way to achieve those objectives. I have no idea what's wrong with a loop. There's a loop inside of `Array.from()` and there's a loop inside of `.reduce()`. So, you're not avoiding loops at all. It still isn't obvious to me what the `.reduce()` loop does at all. – jfriend00 Jul 04 '18 at 00:27
  • 2
    Oh, and there's no comments in the code or in the answer to help explain how anything works. You don't make any effort to "teach" what was wrong with the OP's code or how any of your techniques work. – jfriend00 Jul 04 '18 at 00:29