Add Future.doWhile to dart:async.

Add Future.doWhile to dart:async.

This makes both Future.doWhile and Future.forEach interact more
intuitively with zones. It also explicitly allows the callback to
Future.forEach to return a non-Future value.

BUG=http://dartbug.com/19645, http://dartbug.com/19646
RELNOTE=Add Future.doWhile and allow Future.forEach's callback to return a non-Future.
R=lrn@google.com

Committed: https://code.google.com/p/dart/source/detail?r=37750

[Lasse Reichstein Nielsen, 2014-06-26 06:26:49]

LGTM if the tests run.

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart
File sdk/lib/async/future.dart (right):

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:295: * [f] may return a [Future] or a non-[Future]
value.
Add "The function " first, to not begin the sentence with a lower-case letter.

A "Future or a non-Future" sounds like it can just return anything (well, it
can), but doesn't say what difference it makes.
How about:

If [f] returns a non-future, iteration continues immediately, otherwise it waits
for the returned future to complete.

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:301: return new Future.sync(() =>
f(iterator.current)).then((_) => true);
Maybe:
  return new Future.sync(() { f(iterator.current); return true; });

You can even move this function outside the doWhile, so you don't need to create
a new closure for each element:

Iterator iterator = input.iterator;
var callback = () { f(iterator.current); return true; };
return doWhile(() {
  if (!iterator.moveNext()) return false;
  return new Future.sync(callback);
};

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:315: * [f] may return a [Future<bool>] or a
non-[Future] [bool].
It can return Future<dynamic> or Future<Object> as well, as long as the result
is a bool.
How about:

The function [f] must return either a [bool] value or a [Future] that will
complete with a [bool] value.

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:325: onError: doneSignal._completeError);
Maybe indent this line to the '('?

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:330: nextIteration(keepGoing);
keepGoing -> true ?
Tests? :)

https://codereview.chromium.org/356773005/diff/1/tests/lib/async/futures_test...
File tests/lib/async/futures_test.dart (right):

https://codereview.chromium.org/356773005/diff/1/tests/lib/async/futures_test...
tests/lib/async/futures_test.dart:192: }).then((_) => Expect.listEquals([1, 2,
3, 4, 5], seen));
The entire "(n) { seen.add(n); return; }" function can be replaced by just
"seen.add".

https://codereview.chromium.org/356773005/diff/1/tests/lib/async/futures_test...
tests/lib/async/futures_test.dart:261: Expect.equals(14, list.length);
14 -> 18 ?

[nweiz, 2014-06-26 19:48:12]

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart
File sdk/lib/async/future.dart (right):

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:295: * [f] may return a [Future] or a non-[Future]
value.
On 2014/06/26 06:26:49, Lasse Reichstein Nielsen wrote:
> Add "The function " first, to not begin the sentence with a lower-case letter.
> 
> A "Future or a non-Future" sounds like it can just return anything (well, it
> can), but doesn't say what difference it makes.
> How about:
> 
> If [f] returns a non-future, iteration continues immediately, otherwise it
waits
> for the returned future to complete.

Done.

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:301: return new Future.sync(() =>
f(iterator.current)).then((_) => true);
On 2014/06/26 06:26:49, Lasse Reichstein Nielsen wrote:
> Maybe:
>   return new Future.sync(() { f(iterator.current); return true; });

That doesn't handle the case where f returns a Future.

> You can even move this function outside the doWhile, so you don't need to
create
> a new closure for each element:
> 
> Iterator iterator = input.iterator;
> var callback = () { f(iterator.current); return true; };
> return doWhile(() {
>   if (!iterator.moveNext()) return false;
>   return new Future.sync(callback);
> };

This seems like premature optimization.

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:315: * [f] may return a [Future<bool>] or a
non-[Future] [bool].
On 2014/06/26 06:26:49, Lasse Reichstein Nielsen wrote:
> It can return Future<dynamic> or Future<Object> as well, as long as the result
> is a bool.
> How about:
> 
> The function [f] must return either a [bool] value or a [Future] that will
> complete with a [bool] value.

Done.

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:325: onError: doneSignal._completeError);
On 2014/06/26 06:26:49, Lasse Reichstein Nielsen wrote:
> Maybe indent this line to the '('?

Done.

https://codereview.chromium.org/356773005/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:330: nextIteration(keepGoing);
On 2014/06/26 06:26:49, Lasse Reichstein Nielsen wrote:
> keepGoing -> true ?
> Tests? :)

Done.

https://codereview.chromium.org/356773005/diff/1/tests/lib/async/futures_test...
File tests/lib/async/futures_test.dart (right):

https://codereview.chromium.org/356773005/diff/1/tests/lib/async/futures_test...
tests/lib/async/futures_test.dart:192: }).then((_) => Expect.listEquals([1, 2,
3, 4, 5], seen));
On 2014/06/26 06:26:49, Lasse Reichstein Nielsen wrote:
> The entire "(n) { seen.add(n); return; }" function can be replaced by just
> "seen.add".

Done.

https://codereview.chromium.org/356773005/diff/1/tests/lib/async/futures_test...
tests/lib/async/futures_test.dart:261: Expect.equals(14, list.length);
On 2014/06/26 06:26:49, Lasse Reichstein Nielsen wrote:
> 14 -> 18 ?

Done.

[nweiz, 2014-06-26 19:53:55]

Committed patchset #2 manually as r37750 (presubmit successful).