Add async test func support for whilst/until, deprecate during?

[Qix-]

Alright, few qualms with during and doDuring, along with the synchronous whilst, doWhilst, until and doUntil.

First of all, how is during semantically correct? During means within a duration, notwithstanding iteration (but instead, time).

Secondly, why was during and its sister function chosen over an Function.length === 0 approach for two identical methods whilst and doWhilst (and then left out for the two counterparts, until and doUntil)?

Third, doesn't a synchronous test completely go against the idea of Async to begin with?

---

Why not

• remove/deprecate during and doDuring (i.e. revert during & doDuring with async test #800)
• deprecate the synchronous test forms of whilst, doWhilst, until and doUntil
• add non-breaking functionality to those four methods by performing a Function.length check on the passed function and treating it as async if its length is greater than 0
• ultimately phase out the use of the synchronous check

These methods are severely crippled by breaking the most obvious pattern of this library, and adding copies to fix their problems is a messy solution.

I'd be happy to do a pull request.

[megawac]

I'm in partial agreement, however detecting via Function.length doesn't seem correct -- especially as the test function can receive parameters from the result of the iterator. It would probably more reasonable to do the detection based on the return from the function (i.e. value for sync; none for async) however this would be hacky and run into back-compat issues as well. The only solution I can think of would be to do a /\breturn\b/.test(testFn) to detect synchronicity however this isn't bullet proof either.

I'd be more inclined to do this in v2

[aearly]

Yeah, Function.length is not a good measure -- also because it doesn't work with partially applied functions. async.apply(fs.access, filename) has a length of 0. I don't think there's a reliable way to detect whether a function is meant to be sync or async. How about an optional flag?

async.whilst( testFn, isAsnyc=false, asyncFn, callback)
async.doWhilst(asyncFn, testFn, isAsnyc=false, callback)

[megawac]

I'd rather seperate names async.whilstSync

On Sun, Jul 19, 2015 at 3:15 PM, Alexander Early notifications@github.com
wrote:

Yeah, Function.length is not a good measure -- also because it doesn't
work with partially applied functions. async.apply(fs.access, filename)
has a length of 0. I don't think there's a reliable way to detect whether a
function is meant to be sync or async. How about an optional flag?

async.whilst( testFn, isAsnyc=false, asyncFn, callback)
async.doWhilst(asyncFn, testFn, isAsnyc=false, callback)

—
Reply to this email directly or view it on GitHub
#850 (comment).

[Qix-]

@megawac async.whilstSync is also a naming pattern of other methods, though. I think it'd be confusing since whilst doesn't really have a way to be synchronous like other methods convey themselves to be. Just a thought.

[aearly]

Another thought here: we could make the testFn always async, and then just have people use asyncify to adapt a synchronous function.

[aearly]

I see 3 ways forward here:

1. Leave as is, semantics be damned
2. Rename existing methods, either whilstSync with during becoming whilst, etc.; or whilst staying as-is and during becoming something like whilstAsync.
3. Make the test function always async, during becomes an alias for whilst (or removed).

Personally, I like option 3, seems to make the most sense. Migrating is easy with asyncify (or just using an async function in Node 7+).

[megawac]

I like option 3) as well. I hate having to refer to the documentation to figure out which method to use every damn time