workshopper / learnyounode

Learn You The Node.js For Much Win! An intro to Node.js via a set of self-guided workshops.
Other
7.25k stars 1.84k forks source link

Stuck on [9 of 13] Juggling Async #474

Closed jamiewilson closed 7 years ago

jamiewilson commented 8 years ago

I'm stuck!

My solution sometimes passes verification, but only because—I suspect—the callbacks just happen to match the order in which each httpGet is able to finish. But even with studying the official solution, I've not been able to figure this one out.

Since the prompt suggests to try this without a helper library, it would be nice for the official solution to show that approach as well. Here is where I'm at. Any suggestion where I might be going wrong or ignoring something would be super helpful!

var http = require('http')

// Create empty varaiables to be used later
var compiledData = ''
var results = []
var httpGetRunCount = 0

var startingArg = 2
var numberOfArgs = process.argv.length

// Run 'http.get' for each argument
// Compile returned data into one
// Add 'compiledData' to the 'results' array
// Reset the 'compiledData' variable to empty string
// Keep track of how many times it's run with 'httpGetRunCount'
// If it's run 3 times, print the results
function httpGet(index) {
  http.get(process.argv[index], function(response) {
    response.on('data', function(data) {
      compiledData += data.toString()
    })
    response.on('end', function() {
      results[index] = compiledData
      compiledData = ''
      httpGetRunCount++
      if (httpGetRunCount == 3)
        printResults()
    })
    response.on('error', console.error)
  }).on('error', console.error)
}

// Iterate through array and log each result
function printResults() {
  for (var i = startingArg; i < numberOfArgs; i++)
    console.log(results[i])
}

// Store an index for each 'httpGet' call
// which we'll pass into httpGet as 'index'
for (var i = startingArg; i < numberOfArgs; i++)
  httpGet(i)
matthamil commented 7 years ago

@jamiewilson You would be correct to assume that this code passes only when the HTTP requests happen to complete in the correct order. What's happening here is you are mixing asynchronous code with synchronous code (the for loop). The synchronous code does not wait for the asynchronous code to finish before continuing.

One solution is to wrap each function call of httpGet() in a Promise:

const theArguments = [] // All of the arguments passed into process.argv without [0] and [1]
Promise.all(
    theArguments.map(arg => Promise.resolve(httpGet(arg)))
);

Here's what's happening. Promise.all() takes an array of Promises. Each individual HTTP request is wrapped in a Promise, and then passed into Promise.all(). This lets each promise resolve, and it returns an array of the resolve values from each promise in the order passed to Promise.all().

Somebody correct me if I'm wrong here.

martinheidegger commented 7 years ago

@matthamil Your instruction was spot-on. Thank you. I am closing this issue because it think @jamiewilson managed to continue.