-4

I'm trying to fetch some data, and THEN fetch some other data. Problem is, there's a loop with a timeout in the mix, which makes everything so complicated. And it doesn't work.

In the order, what I'm triying to do:

  1. Loop through listOfGroups and fetch one or many get request to my backend
  2. When that is done, fetch one time another get request

So, here's my code:

var i = 0;
var arrayOfPostURL = [];
isFinished = false;
while (isFinished == false)
    if(i == listOfGroups.length) {
        return fetch("/update_db_fcb_groups/" + theID + "/" + arrayOfPostURL).then((response) => {
                    response.json().then((data) => {
                        this.setState({onFcbGroups: arrayOfPostURL})
                        isFinished = true;
                    });
                });
    }
    if(i<listOfGroups.length){
        setTimeout(function(){
                fetch("/post_to_fcb_group/" + listOfGroups[i] + "/" + theID).then((response) => {
                    // console.log(response);
                    response.json().then((data) => {
                        arrayOfPostURL.push("+" + data.url)
                        i++;
                    });
                });
                // console.log(arr[i])
        },5000)
    }
}

This code even freezes the browser (Google Chrome) !

Any ideas?

mokiliii Lo
  • 587
  • 3
  • 13
  • 26
  • 4
    `while (isFinished == false)` Yes that will freeze your browser. – Keith Feb 16 '18 at 20:47
  • Alright, how do I change that to make it work? – mokiliii Lo Feb 16 '18 at 20:48
  • I'm not. This code is in a React component – mokiliii Lo Feb 16 '18 at 20:51
  • 1
    Possible duplicate of [How do I return the response from an asynchronous call?](https://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call) – Igor Feb 16 '18 at 20:51
  • You should read through the proposed duplicate [How do I return the response from an asynchronous call?](https://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call) (*the answers are well written*) so you gain a fundamental understanding of how to program using asynchronous calls. Once you understand that the changes you need to make to code above become self evident. – Igor Feb 16 '18 at 20:53
  • "while (isFinished == false)" A loop actually triggers every tick of the CPU. This means that you are going to overload any CPU looking at it long enough, since it's the CPU working as fast as it possibly can. This is not the way to do it. It's considered to be very bad practice. You should look into promises and callbacks. If this loop really is a react component, it's a stupid component. – NoobishPro Feb 16 '18 at 20:54

1 Answers1

1

It looks like you're using a while loop when you could be using a for.

var arrayOfPostURL = [];

for (let group of listOfGroups) {
    setTimeout(function() {
        fetch("/post_to_fcb_group/" + group + "/" + theID).then((response) => {
            response.json().then((data) => {
                arrayOfPostURL.push("+" + data.url)
            });
        });
    }, 5000)
}

fetch("/update_db_fcb_groups/" + theID + "/" + arrayOfPostURL).then((response) => {
    response.json().then((data) => {
        this.setState({onFcbGroups: arrayOfPostURL}) 
    }); 
});

Breaking your code down like this reveals a couple other issues.

  1. Your setTimeouts will all finish around the same time. You're just queueing a bunch of fetches that will each take place 5 seconds after they were queued. If you meant to wait 5 seconds between each fetch, this is not the way to do so.
  2. Your final fetch concatenates an array into the URL. That will end up looking something like "/your/url/+url1,+url2". Is that what you intended? It's a fairly unusual URL schema. You might want to change that call to a POST or PUT and pass in a JSON array in the body.
  3. Because you're calling setTimeout on all of your fetches, your final fetch will actually finish when the loop completes which could be before any of the other fetches execute. You likely want to use Promise.all or something similar.
Luke Willis
  • 8,429
  • 4
  • 46
  • 79