skip to Main Content

I’m getting the movie record from the database and the movie record have a list of IDs of the actors in the movie so I’m sending another request to the database to get the actors and then I’m going to replace the list of IDs with the list of the actors. and this code works but of course it’s bad because of setTimeout so how can I execute the code inside setTimeout after the code above finishes

const getMovie = async (req, res) => {
  
    let actors  = [];

    Movie.findById(req.params.id)
    .then(movie => {
        if (movie == null) res.json('This movie does not exist')
        
        else 
        {
           
            movie.Actors.forEach(element => {
                Actor.findById(element)
                .then((a) => {
                    actors.push(a);
                })
            });

            setTimeout(() => {
                movie.Actors = actors;
                res.json(movie);
            }, 1000);

        }
    })
    .catch(err => res.status(400).json(`Error: ${err}`))

}

I tried to use Promise but it didn’t work, but of course I’m an imposter so I may have implemented it in a wrong way

3

Answers


  1. First of all your code works out "luck" because you assume that the all the actors will be fetched from the DB within that 1000ms timeout which might not always be true.

    That being said what you want here is to await for all the promises to complete. The simplest way is the use Javascript’s Promise.all (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all)

    Your code would look something like this:

    const actors = await Promise.all(movie.Actors.map(async actor => {
       return Actor.findById(element)
    }))
    

    And that would fill the actors variable with the results.
    And then you can do:

    movie.Actors = actors;
    res.json(movie);
    

    Of course you need to make this function .then(movie => { async, but that should not affect anything else.

    You could also achieve the same with rxjs (https://rxjs.dev/) but that is a bit more complicated than Promise.all.

    Login or Signup to reply.
  2. You can simply use Promise.all if you’re not afraid of busting your network card’s (or the protocol’s) limits:

    const getMovie = async (req, res) => {
        try {
          const movie = await Movie.findById(req.params.id)
    
          if (!movie) {
            res.json('This movie does not exist')
            return;
          }
    
          movie.Actors = await Promise.all(
            movie.Actors.map(id => Actor.findById(id))
          );
    
          res.json(movie);
        } catch(err) {
          res.status(400).json(`Error: ${err}`);
        }
    }
    
    Login or Signup to reply.
  3. Here’s how I would integrate that logic into your existing promise flow:

    Promise.all(
      movie.Actors.map(element => Actor.findById(element))
    ).then(actors => {
      movie.Actors = actors;
      res.json(movie);
    });
    
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search