Support callback functions with more than one result. #3

Closed
mking opened this Issue Apr 8, 2012 · 5 comments

2 participants

@mking

Some node operations return (err, result1, result2, ...). In this case the results after the first one are dropped.

For instance, if we have fs.read(..., function(err, bytesRead, buffer) {}), only bytesRead is returned.

I'm not sure what the remedy to this is. The easiest thing to do is have results return an array of arrays, but in the common case (fs.stat-like), this adds an unnecessary layer of indirection.

@mbostock
D3 member

Hmm. What about additional arguments to the await callback?

queue()
    .defer(fs.read, …)
    .defer(fs.read, …)
    .await(function(error, bytesReads, buffers) { console.log(bytesReads); });
@mking

If we did that, how would mixing calls work?

queue()
    .defer(fs.fstat, ..)
    .defer(fs.read, ..)
    .await(function(error, arg1s, arg2s) {})

Would arg2s have a hole in it for fs.stat?

I think for multiple calls of the same function (fs.read, fs.read), .await(function(error, bytesReads, buffers) {..}) is nice looking, but for mixed calls it becomes awkward to name the "first results" and "second results" (since the only thing the first array elements have in common is that they are arguments at index 1, etc).

@mbostock
D3 member

A queue is generally intended to be processed in parallel, so it doesn't often make sense to have heterogeneous methods. You could serialize the two calls. Alternatively, you could wrap the functions to make them homogenous.

@mking

I see. I did have a case where I was statting a file and reading the first line from it in parallel, but that kind of parallelization is probably little gain in the scheme of things (compared to parallelizing homogenous calls for an array of files).

In that case I think your proposal is best.

@mbostock mbostock closed this Apr 11, 2012
@mbostock mbostock reopened this Apr 11, 2012
@mbostock
D3 member

To summarize this discussion, I think it’s best to homogenize deferred tasks when needed. So if you have multiple callback values, you'd need to combine them into a single value for access in the await callback. For example:

queue()
    .defer(read, fd1, buffer1, offset1, length1, position1)
    .defer(read, fd2, buffer2, offset2, length2, position2)
    .defer(read, fd3, buffer3, offset3, length3, position3)
    .await(done);

function read(fd, buffer, offset, length, position, callback) {
  fs.read(fd, buffer, offset, length, position, function(error, bytesRead, buffer) {
    callback(error, error ? null : {bytesRead: bytesRead, buffer: buffer});
  });
}

function done(error, results) {
  // results[0].bytesRead, e.g.
}

But I don't think this is the best example since I'm not sure you would ever do something like the above in practice. It might be better to encapsulate the read state of each file a bit better rather than using lots of arguments, in which case you could modify that read state rather than needing to create a new return value from the callback.

And for another reason, reading multiple files in parallel may be slower than reading them sequentially due to disk seek.

@mbostock mbostock closed this Jan 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment