cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Refactoring progress with an unknown number of progress updates #407

Closed cdax closed 9 years ago

cdax commented 9 years ago

I was going through the Refactoring Progress section of the docs, and it'd be great if someone could add an example of refactoring the following (now deprecated) pattern:

function download(url) {
  deferred = when.defer();
  http.get(url, function(res) {
    res.on('data', function(chunk) {
      data += chunk.toString();
      deferred.notify('Downloading: ' + data.length / contentLength + '%');
    });
    res.on('end', function() {
      deferred.resolve('Success: Page downloaded');
    });
    res.on('err', function(err) {
      deferred.reject(err);
    });
  });
  return deferred.promise;
}
...
download(url).then(console.log, console.log, console.log);
...
briancavalier commented 9 years ago

Hey @c-das, great question. There are several options. I'll add a couple here and then maybe we can get them into the docs as well.

The simplest thing is to make download accept the progress function as an argument:

function download(url, onProgress) {
  deferred = when.defer();
  http.get(url, function(res) {
    res.on('data', function(chunk) {
      data += chunk.toString();
      onProgress('Downloading: ' + data.length / contentLength + '%');
    });
    res.on('end', function() {
      deferred.resolve('Success: Page downloaded');
    });
    res.on('err', function(err) {
      deferred.reject(err);
    });
  });
  return deferred.promise;
}
...
download(url, console.log).then(console.log, console.log);
...

For the caller, it's effectively the same. In some cases, if you have really deep call stacks where the progress consumer is far away from the progress producer (ie download in this case), this pattern can require passing the onProgress callback down through several layers. That's not ideal, but in my experience, progress is used infrequently enough where this isn't a big problem.

Another thing you can do is to return a transformed EventEmitter or stream (either a node Stream, or perhaps even a reactive stream, since in this case, you don't care about processing the actual data--just accumulating it).

Here's one example of one way that might work with an EventEmitter:

var EventEmitter = require('events').EventEmitter;

function download(url) {
  var emitter = new EventEmitter();
  http.get(url, function(res) {
    res.on('data', function(chunk) {
      data += chunk.toString();
      emitter.emit('progress', 'Downloading: ' + data.length / contentLength + '%');
    });
    res.on('end', function() {
      emitter.emit('end', 'Success: Page downloaded');
    });
    res.on('err', function(err) {
      emitter.emit('err', err);
    });
  });
  return emitter;
}
...
download(url).on('end', console.log).on('err', console.log).on('progress', console.log);
...

And here's another possibility using reactive stream. A node Stream would work similarly for this use case, but I prefer reactive stream APIs.

var most = require('most');

function download(url) {
  return most.create(function(add, end, err) {
    http.get(url, function(res) {
      res.on('data', add).on('end', end).on('err', error);
    });
  })
  .scan(function(data, chunk) {
    return data + chunk;
  }, data);
}
...
download(url)
  .tap(function(accumulatedDataSoFar) {
    console.log('Downloading: ' + accumulatedDataSoFar.length / contentLength + '%');
  })
  .drain() // Run the stream to completion, returning a promise for the final result
  .then(function(allTheData) {
    console.log('Success: Page downloaded');
  }, console.log);
...

I kind of like that because it externalizes all of the output handling--the download function just downloads. It depends on your situation, of course, as to whether that's what you want or need. Streams can give you much more flexibility, but you may not need that.

OOPS: The promise returned from a scan in the current version of most.js doesn't fulfill with the final scan value--call it a bug. It will in an upcoming patch release, tho :)

I hope those patterns help, or at least give you a good starting point!

briancavalier commented 9 years ago

Ping @c-das. Can we close this, or would you like to discuss further?