Skip to content

Commit 561c366

Browse files
refactor: simplify lengthBy and lengthBeforeMax using while!
Replace the pattern of: - one unconditional MoveNextAsync call before the loop - a mutable 'go' flag - manual go/step tracking ...with the simpler 'while! e.MoveNextAsync() do' form. For lengthBy (all three predicate variants), the new implementation is cleaner and reads more naturally: iterate over elements, count those that match. For lengthBeforeMax, the new implementation advances the enumerator inside the loop body and guards with 'go && i < max', eliminating the pre-loop advance. A beneficial side effect: the old implementation had a read-ahead of 1 (an extra MoveNextAsync call before the loop), which meant lengthOrMax 5 on a 10-element source would evaluate 6 elements instead of 5, and lengthOrMax 0 would still evaluate the first element. The new implementation has no read-ahead, so lengthOrMax N evaluates exactly N elements as expected. Tests updated to reflect the improved (no read-ahead) behaviour, replacing the N+1 and 1 assertions with the correct N and 0. All 5013 existing tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6fd41af commit 561c366

File tree

3 files changed

+18
-36
lines changed

3 files changed

+18
-36
lines changed

release-notes.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
Release notes:
33

44
1.0.0
5+
- refactor: simplify lengthBy and lengthBeforeMax to use while! and remove the redundant mutable 'go' and initial MoveNextAsync
56
- adds TaskSeq.distinctUntilChangedWith and TaskSeq.distinctUntilChangedWithAsync, #345
67
- adds TaskSeq.withCancellation, #167
78
- adds TaskSeq.replicateInfinite, replicateInfiniteAsync, replicateUntilNoneAsync, #345

src/FSharp.Control.TaskSeq.Test/TaskSeq.Length.Tests.fs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,7 @@ module SideEffects =
300300

301301
[<Theory; ClassData(typeof<TestSideEffectTaskSeq>)>]
302302
let ``TaskSeq-lengthOrMax returns max and stops evaluation when sequence exceeds max`` variant = task {
303-
// source has 10 items; max=5 → should stop early
304-
// NOTE: the implementation reads one element ahead (N+1 total MoveNextAsync calls for result N)
303+
// source has 10 items; max=5 → should stop early after exactly 5 elements
305304
let mutable evaluated = 0
306305

307306
let ts = taskSeq {
@@ -312,14 +311,12 @@ module SideEffects =
312311

313312
let! len = ts |> TaskSeq.lengthOrMax 5
314313
len |> should equal 5
315-
// exactly max+1 elements are pulled from the source due to read-ahead
316-
evaluated |> should equal 6
314+
// exactly max elements are pulled from the source
315+
evaluated |> should equal 5
317316
}
318317

319318
[<Fact>]
320-
let ``TaskSeq-lengthOrMax stops evaluating source after reaching max - read-ahead characteristic`` () = task {
321-
// NOTE: the implementation calls MoveNextAsync once before the while loop,
322-
// then once more per iteration. For max=N and a longer source, this means N+1 calls total.
319+
let ``TaskSeq-lengthOrMax stops evaluating source after reaching max`` () = task {
323320
let mutable sideEffects = 0
324321

325322
let ts = taskSeq {
@@ -330,14 +327,12 @@ module SideEffects =
330327

331328
let! len = ts |> TaskSeq.lengthOrMax 7
332329
len |> should equal 7
333-
// max+1 elements are evaluated due to the read-ahead implementation pattern
334-
sideEffects |> should equal 8
330+
// exactly max elements are evaluated
331+
sideEffects |> should equal 7
335332
}
336333

337334
[<Fact>]
338-
let ``TaskSeq-lengthOrMax with max=0 still evaluates the first element due to read-ahead`` () = task {
339-
// The implementation unconditionally calls MoveNextAsync once before entering
340-
// the while loop, so even max=0 evaluates the first element of the source.
335+
let ``TaskSeq-lengthOrMax with max=0 evaluates zero elements`` () = task {
341336
let mutable sideEffects = 0
342337

343338
let ts = taskSeq {
@@ -349,6 +344,6 @@ module SideEffects =
349344

350345
let! len = ts |> TaskSeq.lengthOrMax 0
351346
len |> should equal 0
352-
// one element was evaluated despite max=0
353-
sideEffects |> should equal 1
347+
// no elements evaluated when max=0
348+
sideEffects |> should equal 0
354349
}

src/FSharp.Control.TaskSeq/TaskSeqInternal.fs

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -179,37 +179,25 @@ module internal TaskSeqInternal =
179179
checkNonNull (nameof source) source
180180

181181
task {
182-
183182
use e = source.GetAsyncEnumerator CancellationToken.None
184-
let mutable go = true
185183
let mutable i = 0
186-
let! step = e.MoveNextAsync()
187-
go <- step
188184

189185
match predicate with
190186
| None ->
191-
while go do
192-
let! step = e.MoveNextAsync()
193-
i <- i + 1 // update before moving: we are counting, not indexing
194-
go <- step
187+
while! e.MoveNextAsync() do
188+
i <- i + 1
195189

196190
| Some(Predicate predicate) ->
197-
while go do
191+
while! e.MoveNextAsync() do
198192
if predicate e.Current then
199193
i <- i + 1
200194

201-
let! step = e.MoveNextAsync()
202-
go <- step
203-
204195
| Some(PredicateAsync predicate) ->
205-
while go do
196+
while! e.MoveNextAsync() do
206197
match! predicate e.Current with
207198
| true -> i <- i + 1
208199
| false -> ()
209200

210-
let! step = e.MoveNextAsync()
211-
go <- step
212-
213201
return i
214202
}
215203

@@ -219,15 +207,13 @@ module internal TaskSeqInternal =
219207

220208
task {
221209
use e = source.GetAsyncEnumerator CancellationToken.None
222-
let mutable go = true
223210
let mutable i = 0
224-
let! step = e.MoveNextAsync()
225-
go <- step
211+
let mutable go = true
226212

227213
while go && i < max do
228-
i <- i + 1 // update before moving: we are counting, not indexing
229-
let! step = e.MoveNextAsync()
230-
go <- step
214+
let! hasMore = e.MoveNextAsync()
215+
216+
if hasMore then i <- i + 1 else go <- false
231217

232218
return i
233219
}

0 commit comments

Comments
 (0)