Add someLimit and everyLimit

[megawac]

Merges the implementations of some and every and adds their respective limit functions (with partial short circuiting)

PS I think short circuiting should be done like lodash does it (allowing each to break):
if cb resolves with cb(false) you break the loop (in lodash if you return false it breaks the loop)

Resolves #548

[megawac]

gyeates at boompad in ~/code/async on arr-testers [!$]
$ ./perf/benchmark.js -g some
Comparing 1.2.1 with current on Node v0.12.0
--------------------------------------
some - no short circuit- false(500) 1.2.1 x 1,838 ops/sec ±5.41% (23 runs sampled), 0.544ms per run
some - no short circuit- false(500) current x 1,899 ops/sec ±4.72% (25 runs sampled), 0.527ms per run
Tie
--------------------------------------
some - short circuit - true(500) 1.2.1 x 1,598 ops/sec ±10.93% (25 runs sampled), 0.626ms per run
some - short circuit - true(500) current x 2,007 ops/sec ±5.35% (26 runs sampled), 0.498ms per run
current is faster
--------------------------------------
current faster overall (1.02ms total vs. 1.17ms total)
current won more benchmarks (1 vs. 0)

gyeates at boompad in ~/code/async on arr-testers [!$]
$ ./perf/benchmark.js -g every
Comparing 1.2.1 with current on Node v0.12.0
--------------------------------------
every - no short circuit- true(500) 1.2.1 x 1,388 ops/sec ±14.84% (18 runs sampled), 0.72ms per run
every - no short circuit- true(500) current x 1,634 ops/sec ±6.52% (21 runs sampled), 0.612ms per run
current is faster
--------------------------------------
every - short circuit - false(500) 1.2.1 x 1,828 ops/sec ±5.13% (26 runs sampled), 0.547ms per run
every - short circuit - false(500) current x 1,682 ops/sec ±6.95% (24 runs sampled), 0.595ms per run
Tie
--------------------------------------
current faster overall (1.21ms total vs. 1.27ms total)
current won more benchmarks (1 vs. 0)

[aearly]

I've thought a lot about the best way to short circuit in the more general case. cb(false) seems to generic for me -- most libraries just check for truthiness of the err value, so false might be a non-error, non-short-circuiting value for some people.

I've thought about defining a constant like async.BREAK = {} as a sort of a stop sentinel (or a poor man's Symbol). You would pass this as the error in a async.eachOf iterator callback, and then the final callback for eachOf would handle it accordingly:

function done(err) {
  if (err) {
    return callback(err === async.BREAK ? null : err);
  }
  //..
}

Seems a bit hackish, but I haven't thought of something better. This type of strategy would also help in things like auto or waterfall where you want to exit early, but you still need to cancel the remaining steps in the sequence to prevent memory leaks.

[aearly]

But 👍 for this otherwise, just waiting for CI.

[aearly]

Should we add cb = cb || noop to prevent having to check for this every time?

[megawac]

This is only done at the last step, noop actually makes it more complicated because then at the other step (iterator) it has to check if (cb === noop)

[megawac]

I've thought about defining a constant like async.BREAK = {} as a sort of a stop sentinel (or a poor man's Symbol).

Yeah, thats what underscore used to do and makes sense if false doesn't work

[aearly]

Yeah, thats what underscore used to do

Do you know why underscore moved away from it?

[megawac]

@aearly should I add a unit test to ensure some and every short circuit?

[aearly]

Yes -- always best to have tests for all the corner cases wherever possible.

[megawac]

Do you know why underscore moved away from it?

Yeah, for performance reasons we implemented it as a for loop jashkenas/underscore#1710

[megawac]

Should I add some unit tests to ensure they short circuit? (ps minor breaking change) Whoops, dementia setting in

[aearly]

Ahh, I see --- it wasn't like lodash where it has a generic way to break, it was a special behavior for _.some and _.every

[aearly]

Why don't we save the short circuiting for 2.0 -- you're right -- it will have minor breaking changes.

[megawac]

IDK, I think the current behaviour can also be classified as a bug as people usually expect some and every to exit immediately (the only breaking change is the test function is called less), whereas the finalCallback is only ever called once

[megawac]

This can also be used to implement detect, however I'll do that in a seperate PR

[aearly]

Actually, I realize short circuiting isn't breaking at all. Since we're using the parallel _.eachOf to implement every, the iterator will be called for every element regardless. Short circuiting only applies to these new everyLimit variations.

[megawac]

Nope, some and every also short circuit because of this line https://github.com/caolan/async/pull/828/files#diff-688ede0ee2da1fa3d89f2fc485f39273R461

[megawac]

Wrong line https://github.com/caolan/async/pull/828/files#diff-688ede0ee2da1fa3d89f2fc485f39273R464

[aearly]

They exit early, but the iterators are still called for every element in the array. The callbacks are just ignored once the relevant true or false is encountered.

[aearly]

Unless we're talking about a different subtle breaking change.

[megawac]

For synchronous iterators, iterator will only ever be called once as eachOf doesn't clal ensure sync AFAIK

[aearly]

Well, this test didn't break, so I think we're okay.

[megawac]

It'll break if you remove the setTimeout :)

[charlierudolph]

In #809, I updated how the docs are formatted where the series and limit functions no longer need their own documentation. Instead the series/limit is documented as whole and then the related commands are just mentioned with the documentation of the normal command.

These lines should become

* [`some`](#some), `someLimit`
* [`every`](#every), `everyLimit`
* [`concat`](#concat), `concatSeries`

And the documentation for someLimit should be removed and the following should be added to the some documentation.

__Related__

* someLimit(arr, limit, iterator, [callback])

Same for every.

[megawac]

Hi @charlierudolph, if you have some time would you mind submitting a pull request with your suggested documentation changes?