Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider adding queue.deferAll. #8

Closed
natevw opened this issue Feb 8, 2013 · 6 comments
Closed

Consider adding queue.deferAll. #8

natevw opened this issue Feb 8, 2013 · 6 comments

Comments

@natevw
Copy link

natevw commented Feb 8, 2013

[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
Copy link
Member

mbostock commented Feb 8, 2013

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
Copy link
Member

mbostock commented Feb 8, 2013

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

@natevw
Copy link
Author

natevw commented Feb 9, 2013

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
Copy link
Member

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!

@natevw
Copy link
Author

natevw commented Apr 3, 2013

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
Copy link
Member

mbostock commented Apr 3, 2013

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
Labels
None yet
Development

No branches or pull requests

2 participants