Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Consider adding queue.deferAll. #8

Closed
natevw opened this Issue · 6 comments

2 participants

@natevw

[Original subject: forEach-ish defer]

IMO adding queue.deferAll(array, method) to match .awaitAll would not harm this library's (greatly appreciated!) minimalism:

queue(N).deferAll(input, work).awaitAll(function (e, output) {
    …
});

vs.

var q = queue(N);
input.forEach(function (d) { q.defer(work, d); });
q.awaitAll(function (e, output) {
    …
});
@mbostock
Owner

That's an interesting idea. But I think I'd probably swap the order of the arguments to match queue.defer more closely:

queue(1).deferAll(d3.json, ["foo.json", "bar.json"]).awaitAll(function(error, jsons) {
  …
});
@mbostock
Owner

Also, if you have multiple arguments to your defer method, you pass multiple arrays? That's a bit awkward.

@natevw

I considered swapping the arguments, and still not terribly opposed. I went with the general "function blocks last" etiquette, reasoning that it better highlighted the queue's role in providing the "rest" (in this case all) of the arguments to the method call. The resulting symmetry versus the awaitAll call also seemed nice, but not opposed to switching back.

Not following the multiple arguments question. Seems best to leave out any zipping or index passing if that's what you mean. Implicit in my example was that "work" should always expect two parameters (item, callback) just as the await/awaitAll result-gather callback only gathers a single item beyond the error.

@mbostock
Owner

Re. multiple arguments, say I have an asynchronous function:

function paint(object, color, callback) {
  …
}

With the current queue API, I can do something like:

queue()
    .defer(paint, fence, "white")
    .defer(paint, door, "blue")
    .defer(paint, walls, "brown")
    .await(done);

What would you do in the case of deferAll? It seems like there are two options. Option A is to pass an array of arrays, where each element in the array is the arguments for the asynchronous function:

queue.deferAll(paint, [[fence, "white"], [door, "blue"], [walls, "brown"]]).awaitAll(done);

Option B is to pass multiple arguments, which has the nice property of simplifying the common case where the deferred function takes only a single argument:

queue.deferAll(paint, [fence, door, walls], ["white", "blue", "brown"]).awaitAll(done);

However, it has the undesirable property that it separates arguments across arrays (it’s less obvious which object gets which color when reading the code), and it’s further awkward if you want to pass a variable number of arguments to your asynchronous function (you might want a sparse array, such as ["white",, "brown"]).

Given this ambiguity, and my general tendency to favor parsimony, I’d rather not add a deferAll function as this moment. Hope that seems reasonable, and thanks for the suggestion!

@mbostock mbostock closed this
@natevw

Makes sense now, yes…

Option A isn't really an option. This library shouldn't be peering into the mapped-over container's items, or trying to make apply vs. call guesses based on the called task's arity — how would you handle a different sort of paint function which expected to get an array object as its first parameter?

Option B is correct. I think in practice it would be really ugly 50% of the time (when the data is already nested within subarrays…then you'd be better off wrapping/rewriting your tasks to expand the arguments Option A–style, which is still kinda ugly) and ideal 50% of the time (where two arrays needed zipping anyway).

I still like the idea, but can write a helper or patch an experimental one in to try it first. If I end up needing and liking it enough, I'll send a pull request…in the meantime +1 for parsimony :-)

@mbostock
Owner

Option A doesn’t require looking at function.length or guessing; if you wanted to pass an array object, you’d need do something like:

queue().deferAll(paint, [[[fence, door, walls]]]).awaitAlll(done);

Given:

function paint(array) {
  …
}

But that’s a lot of brackets!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.