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

Feature Request: Foreach with the object key please #26

Closed
Pita opened this issue Apr 30, 2011 · 4 comments
Closed

Feature Request: Foreach with the object key please #26

Pita opened this issue Apr 30, 2011 · 4 comments

Comments

@Pita
Copy link

Pita commented Apr 30, 2011

Hi,

I realy like your module cause it makes my life much easier :)

But, following example:

var object = {"a":"fooA", "b":"fooB"}

async.forEach(object, function(item, callback){
  //why is here no way to access the key of the item?!
}, callback);
@caolan
Copy link
Owner

caolan commented Apr 30, 2011

Ultimately, the problem is the long list of arguments the callbacks would need to support, every callback would look like:

function (obj, index, arr, callback) { ... }

Even though in most circumstances you wouldn't need to use the additional parameters.

One solution is to detect the number of arguments the callback function accepts and just pass that number of variables to it, always providing the callback as the last one. This is something I've been exploring, see the nimble project for an example of how that might work: http://caolan.github.com/nimble

And for an explanation of the technique: http://caolanmcmahon.com/posts/flexible_callback_arguments

It seems to be working nicely so far. Provided I don't run into any major issues with it I may add it to the async library. Unfortunately it would mean some backwards-incompatible changes, so it would need to be on the next major version.

For now, perhaps nimble would work for you?

@rgabo
Copy link

rgabo commented May 24, 2011

@caolan, nimble does indeed iterate over object values, but async's forEach[Series] doesn't. Any reason why this is the case? Also seen issue #13 point this out directly.

It seems we're only using the 'nimble' subset of async so we might eventually just switch. While it's not explicitly documented on http://caolan.github.com/nimble, all functional constructs (map, filter, each, etc) do support a final callback. The minified size is quite impressive :)

UPDATE: we're actually using waterfall and whilst / until so nimble wouldn't be enough by itself. Would a pull request for #13 help this get into async faster? No backwards compatibility issues for iterating over the values of an object.

UPDATE2: nevermind, we do need the keys as well, so resorting to nimble for now. +1 for checking the arity of the callback function and using flexible callback arguments in the next major version. Any release roadmap? :)

@beatgammit
Copy link

I like the way the module forEachAsync does it. The callback is always the first parameter in the callback for each element in the array.

forEachAsync(arr, function (next, item, idx, arr) {
    // just like Array.forEachAsync, except with an extra first parameter
}.then(function () {
    // called after everything is done
});

I don't however, like the then part, and I would prefer to use a third parameter, just like async does.

The issue is, this would totally break backwards compat, and make this module inconsistent with the rest of async's methods.

Is this a big enough deal to warrant a major API change?

@brianmaissy
Copy link
Contributor

This is a duplicate of #168 and should be closed.

@caolan caolan closed this as completed Mar 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants