From 624e07bdbf90f81cf98d1c8b153a0221b06d26ac Mon Sep 17 00:00:00 2001 From: Ruben Bartelink Date: Sat, 23 Dec 2023 17:58:10 +0000 Subject: [PATCH 1/3] refactor(skip+takeWhile): Make impls consistent --- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 161 ++++++------------ 1 file changed, 56 insertions(+), 105 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index a062acd..b255023 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -802,56 +802,38 @@ module internal TaskSeqInternal = taskSeq { use e = source.GetAsyncEnumerator CancellationToken.None let! notEmpty = e.MoveNextAsync() - let mutable more = notEmpty + let mutable cont = notEmpty - match whileKind, predicate with - | Exclusive, Predicate predicate -> // takeWhile - while more do - let value = e.Current - more <- predicate value + let inclusive = + match whileKind with + | Inclusive -> true + | Exclusive -> false - if more then - // yield ONLY if predicate is true - yield value - let! hasMore = e.MoveNextAsync() - more <- hasMore - - | Inclusive, Predicate predicate -> // takeWhileInclusive - while more do - let value = e.Current - more <- predicate value - - // yield regardless of result of predicate - yield value - - if more then + match predicate with + | Predicate predicate -> // takeWhile(Inclusive)? + while cont do + if predicate e.Current then + yield e.Current let! hasMore = e.MoveNextAsync() - more <- hasMore + cont <- hasMore + else + if inclusive then + yield e.Current - | Exclusive, PredicateAsync predicate -> // takeWhileAsync - while more do - let value = e.Current - let! passed = predicate value - more <- passed + cont <- false - if more then - // yield ONLY if predicate is true - yield value + | PredicateAsync predicate -> // takeWhile(Inclusive)?Async + while cont do + match! predicate e.Current with + | true -> + yield e.Current let! hasMore = e.MoveNextAsync() - more <- hasMore - - | Inclusive, PredicateAsync predicate -> // takeWhileInclusiveAsync - while more do - let value = e.Current - let! passed = predicate value - more <- passed - - // yield regardless of predicate - yield value + cont <- hasMore + | false -> + if inclusive then + yield e.Current - if more then - let! hasMore = e.MoveNextAsync() - more <- hasMore + cont <- false } let skipWhile whileKind predicate (source: TaskSeq<_>) = @@ -859,77 +841,46 @@ module internal TaskSeqInternal = taskSeq { use e = source.GetAsyncEnumerator CancellationToken.None - let! moveFirst = e.MoveNextAsync() - let mutable more = moveFirst - - match whileKind, predicate with - | Exclusive, Predicate predicate -> // skipWhile - while more && predicate e.Current do - let! hasMore = e.MoveNextAsync() - more <- hasMore - - if more then - // yield the last one where the predicate was false - // (this ensures we skip 0 or more) - yield e.Current - - while! e.MoveNextAsync() do // get the rest - yield e.Current - - | Inclusive, Predicate predicate -> // skipWhileInclusive - while more && predicate e.Current do - let! hasMore = e.MoveNextAsync() - more <- hasMore - - if more then - // yield the rest (this ensures we skip 1 or more) - while! e.MoveNextAsync() do - yield e.Current - - | Exclusive, PredicateAsync predicate -> // skipWhileAsync - let mutable cont = true - - if more then - let! hasMore = predicate e.Current - cont <- hasMore - - while more && cont do - let! moveNext = e.MoveNextAsync() - - if moveNext then - let! hasMore = predicate e.Current - cont <- hasMore - more <- moveNext - - if more then - // yield the last one where the predicate was false - // (this ensures we skip 0 or more) - yield e.Current + match! e.MoveNextAsync() with + | false -> () // Nothing further to do, no matter what the rules are + | true -> - while! e.MoveNextAsync() do // get the rest - yield e.Current + let exclusive = + match whileKind with + | Exclusive -> true + | Inclusive -> false - | Inclusive, PredicateAsync predicate -> // skipWhileInclusiveAsync let mutable cont = true - if more then - let! hasMore = predicate e.Current - cont <- hasMore + match predicate with + | Predicate predicate -> // skipWhile(Inclusive)? + while cont do + if predicate e.Current then // spam -> skip + let! hasAnother = e.MoveNextAsync() + cont <- hasAnother + else // Starting the ham + if exclusive then + yield e.Current // return the item as it does not meet the condition for skipping - while more && cont do - let! moveNext = e.MoveNextAsync() + while! e.MoveNextAsync() do // propagate the rest + yield e.Current - if moveNext then - let! hasMore = predicate e.Current - cont <- hasMore + cont <- false + | PredicateAsync predicate -> // skipWhile(Inclusive)?Async + while cont do + match! predicate e.Current with + | true -> + let! hasAnother = e.MoveNextAsync() + cont <- hasAnother + | false -> // We're starting the ham + if exclusive then + yield e.Current // return the item as it does not meet the condition for skipping - more <- moveNext + while! e.MoveNextAsync() do // propagate the rest + yield e.Current - if more then - // get the rest, this gives 1 or more semantics - while! e.MoveNextAsync() do - yield e.Current + cont <- false } // Consider turning using an F# version of this instead? From 4ed6ba52b377d1115cb65cf2748fac2f78cd1dab Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Fri, 15 Mar 2024 16:13:49 +0100 Subject: [PATCH 2/3] Simplify `takeWhile` and `takeWhileInclusive`, easier to follow --- src/FSharp.Control.TaskSeq/TaskSeq.fs | 8 ++-- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 48 ++++++++----------- 2 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index 7143446..6fc2da8 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -299,10 +299,10 @@ type TaskSeq private () = static member take count source = Internal.skipOrTake Take count source static member truncate count source = Internal.skipOrTake Truncate count source - static member takeWhile predicate source = Internal.takeWhile Exclusive (Predicate predicate) source - static member takeWhileAsync predicate source = Internal.takeWhile Exclusive (PredicateAsync predicate) source - static member takeWhileInclusive predicate source = Internal.takeWhile Inclusive (Predicate predicate) source - static member takeWhileInclusiveAsync predicate source = Internal.takeWhile Inclusive (PredicateAsync predicate) source + static member takeWhile predicate source = Internal.takeWhile false (Predicate predicate) source + static member takeWhileAsync predicate source = Internal.takeWhile false (PredicateAsync predicate) source + static member takeWhileInclusive predicate source = Internal.takeWhile true (Predicate predicate) source + static member takeWhileInclusiveAsync predicate source = Internal.takeWhile true (PredicateAsync predicate) source static member skipWhile predicate source = Internal.skipWhile Exclusive (Predicate predicate) source static member skipWhileAsync predicate source = Internal.skipWhile Exclusive (PredicateAsync predicate) source static member skipWhileInclusive predicate source = Internal.skipWhile Inclusive (Predicate predicate) source diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index b255023..2ccabba 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -796,44 +796,36 @@ module internal TaskSeqInternal = } - let takeWhile whileKind predicate (source: TaskSeq<_>) = + let takeWhile isInclusive predicate (source: TaskSeq<_>) = checkNonNull (nameof source) source taskSeq { use e = source.GetAsyncEnumerator CancellationToken.None let! notEmpty = e.MoveNextAsync() - let mutable cont = notEmpty - - let inclusive = - match whileKind with - | Inclusive -> true - | Exclusive -> false + let mutable hasMore = notEmpty match predicate with - | Predicate predicate -> // takeWhile(Inclusive)? - while cont do - if predicate e.Current then + | Predicate synchronousPredicate -> + while hasMore && synchronousPredicate e.Current do + yield e.Current + let! cont = e.MoveNextAsync() + hasMore <- cont + + | PredicateAsync asyncPredicate -> + let mutable predicateHolds = true + while hasMore && predicateHolds do + let! predicateIsTrue = asyncPredicate e.Current + if predicateIsTrue then yield e.Current - let! hasMore = e.MoveNextAsync() - cont <- hasMore - else - if inclusive then - yield e.Current - - cont <- false + let! cont = e.MoveNextAsync() + hasMore <- cont - | PredicateAsync predicate -> // takeWhile(Inclusive)?Async - while cont do - match! predicate e.Current with - | true -> - yield e.Current - let! hasMore = e.MoveNextAsync() - cont <- hasMore - | false -> - if inclusive then - yield e.Current + predicateHolds <- predicateIsTrue - cont <- false + // "inclusive" means: always return the item that we pulled, regardless of the result of applying the predicate + // and only stop thereafter. The non-inclusive versions, in contrast, do not return the item under which the predicate is false. + if hasMore && isInclusive then + yield e.Current } let skipWhile whileKind predicate (source: TaskSeq<_>) = From 56f4dbe1b88ea58843bb5252bea5f30aa55cf775 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Fri, 15 Mar 2024 16:36:41 +0100 Subject: [PATCH 3/3] Also simplify `skipWhile` and `skipWhileInclusive` (plus async variants) --- src/FSharp.Control.TaskSeq/TaskSeq.fs | 8 +-- src/FSharp.Control.TaskSeq/TaskSeqInternal.fs | 70 ++++++++----------- 2 files changed, 32 insertions(+), 46 deletions(-) diff --git a/src/FSharp.Control.TaskSeq/TaskSeq.fs b/src/FSharp.Control.TaskSeq/TaskSeq.fs index 6fc2da8..520b3a3 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeq.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeq.fs @@ -303,10 +303,10 @@ type TaskSeq private () = static member takeWhileAsync predicate source = Internal.takeWhile false (PredicateAsync predicate) source static member takeWhileInclusive predicate source = Internal.takeWhile true (Predicate predicate) source static member takeWhileInclusiveAsync predicate source = Internal.takeWhile true (PredicateAsync predicate) source - static member skipWhile predicate source = Internal.skipWhile Exclusive (Predicate predicate) source - static member skipWhileAsync predicate source = Internal.skipWhile Exclusive (PredicateAsync predicate) source - static member skipWhileInclusive predicate source = Internal.skipWhile Inclusive (Predicate predicate) source - static member skipWhileInclusiveAsync predicate source = Internal.skipWhile Inclusive (PredicateAsync predicate) source + static member skipWhile predicate source = Internal.skipWhile false (Predicate predicate) source + static member skipWhileAsync predicate source = Internal.skipWhile false (PredicateAsync predicate) source + static member skipWhileInclusive predicate source = Internal.skipWhile true (Predicate predicate) source + static member skipWhileInclusiveAsync predicate source = Internal.skipWhile true (PredicateAsync predicate) source static member tryPick chooser source = Internal.tryPick (TryPick chooser) source static member tryPickAsync chooser source = Internal.tryPick (TryPickAsync chooser) source diff --git a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs index 2ccabba..c48821f 100644 --- a/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs +++ b/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs @@ -11,13 +11,6 @@ type internal AsyncEnumStatus = | WithCurrent | AfterAll -[] -type internal WhileKind = - /// The item under test is included (or skipped) even when the predicate returns false - | Inclusive - /// The item under test is always excluded (or not skipped) - | Exclusive - [] type internal TakeOrSkipKind = /// use the Seq.take semantics, raises exception if not enough elements @@ -813,8 +806,10 @@ module internal TaskSeqInternal = | PredicateAsync asyncPredicate -> let mutable predicateHolds = true - while hasMore && predicateHolds do + + while hasMore && predicateHolds do // TODO: check perf if `while!` is going to be better or equal let! predicateIsTrue = asyncPredicate e.Current + if predicateIsTrue then yield e.Current let! cont = e.MoveNextAsync() @@ -828,51 +823,42 @@ module internal TaskSeqInternal = yield e.Current } - let skipWhile whileKind predicate (source: TaskSeq<_>) = + let skipWhile isInclusive predicate (source: TaskSeq<_>) = checkNonNull (nameof source) source taskSeq { use e = source.GetAsyncEnumerator CancellationToken.None + let! notEmpty = e.MoveNextAsync() + let mutable hasMore = notEmpty - match! e.MoveNextAsync() with - | false -> () // Nothing further to do, no matter what the rules are - | true -> - - let exclusive = - match whileKind with - | Exclusive -> true - | Inclusive -> false + match predicate with + | Predicate synchronousPredicate -> + while hasMore && synchronousPredicate e.Current do + // keep skipping + let! cont = e.MoveNextAsync() + hasMore <- cont - let mutable cont = true + | PredicateAsync asyncPredicate -> + let mutable predicateHolds = true - match predicate with - | Predicate predicate -> // skipWhile(Inclusive)? - while cont do - if predicate e.Current then // spam -> skip - let! hasAnother = e.MoveNextAsync() - cont <- hasAnother - else // Starting the ham - if exclusive then - yield e.Current // return the item as it does not meet the condition for skipping + while hasMore && predicateHolds do // TODO: check perf if `while!` is going to be better or equal + let! predicateIsTrue = asyncPredicate e.Current - while! e.MoveNextAsync() do // propagate the rest - yield e.Current + if predicateIsTrue then + // keep skipping + let! cont = e.MoveNextAsync() + hasMore <- cont - cont <- false - | PredicateAsync predicate -> // skipWhile(Inclusive)?Async - while cont do - match! predicate e.Current with - | true -> - let! hasAnother = e.MoveNextAsync() - cont <- hasAnother - | false -> // We're starting the ham - if exclusive then - yield e.Current // return the item as it does not meet the condition for skipping + predicateHolds <- predicateIsTrue - while! e.MoveNextAsync() do // propagate the rest - yield e.Current + // "inclusive" means: always skip the item that we pulled, regardless of the result of applying the predicate + // and only stop thereafter. The non-inclusive versions, in contrast, do not skip the item under which the predicate is false. + if hasMore && not isInclusive then + yield e.Current // don't skip, unless inclusive - cont <- false + // propagate the rest + while! e.MoveNextAsync() do + yield e.Current } // Consider turning using an F# version of this instead?