Implement `TaskSeq.insertAt`, `updateAt`, `removeAt`, `insertManyAt`, `removeManyAt` #236

abelbraaksma · 2024-03-16T00:26:54Z

Part of the push for good coverage of surface area functions, see #208 for an overview. This implements insertAt, updateAt, removeAt, insertManyAt, removeManyAt and maxByAsync.

We won't be implementing updateManyAt unless there's a strong use case. We skip it for the same reasons it was skipped in FSharp.Core for Seq: fsharp/fslang-suggestions#1047 (comment).

Each of these behave exactly like their Seq counterparts:

raises on null input
raises for negative indices
raises when the index turns out to be out of bounds

As before, the xml doc blibs have been taken from seq.fs and modified a bit for readability and applicability to TaskSeq.

The signatures are as follows:

static member insertAt: index: int -> value: 'T -> source: TaskSeq<'T> -> TaskSeq<'T>
static member insertManyAt: index: int -> values: TaskSeq<'T> -> source: TaskSeq<'T> -> TaskSeq<'T>

static member removeAt: index: int -> source: TaskSeq<'T> -> TaskSeq<'T>
static member removeManyAt: index: int -> count: int -> source: TaskSeq<'T> -> TaskSeq<'T>

static member updateAt: index: int -> value: 'T -> source: TaskSeq<'T> -> TaskSeq<'T>

TODO list:

abelbraaksma · 2024-03-16T02:37:11Z

This is ready. Now all that's left is forall and the release notes, and then we can publish the new version finally.

bartelink

Drinking in the completeness of your tests always just fills me with self loathing

Luckily that pain can fuel nit picking and general devil's advocate "just asking questions" routine.

In other words, please take as little or as much of this as you wish, especially given the review is long after the work...

bartelink · 2024-03-16T19:37:56Z

src/FSharp.Control.TaskSeq/TaskSeq.fsi

@@ -1275,3 +1276,65 @@ type TaskSeq =
    /// <exception cref="T:ArgumentNullException">Thrown when the input task sequence is null.</exception>
    static member foldAsync:
        folder: ('State -> 'T -> #Task<'State>) -> state: 'State -> source: TaskSeq<'T> -> Task<'State>
+
+    /// <summary>
+    /// Return a new task sequence with a new item inserted before the given index.


ye olde yields vs returns thing triggering in my head
Yes, all functions return things
the <returns> expresses it in that way and it's clear.
But in general mapping a sequence that's lazy to another thing that's still lazy
Thoughts? If there is prior art art/or it makes any sense to you I might do a walk to see if such language makes sense to apply across the board for all functions that return some mapped/altered rendition of some input?

a new item -> an additional item?
or
a specified item
/ value?

bartelink · 2024-03-16T19:38:29Z

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

+    let ``TaskSeq-updateAt(0) will execute side effects at start of sequence`` () =
+        // NOTE: while not strictly necessary, this mirrors behavior of Seq.updateAt
+
+        let mutable x = 42 // for this test, the potential mutation should not actually occur


bartelink · 2024-03-16T19:39:30Z

src/FSharp.Control.TaskSeq/TaskSeq.fsi

+    /// Return a new task sequence with a new item inserted before the given index.
+    /// </summary>
+    ///
+    /// <param name="index">The index where the item should be inserted.</param>


where -> at which ?

bartelink · 2024-03-16T19:40:31Z

src/FSharp.Control.TaskSeq/TaskSeq.fsi

+    /// <param name="source">The input task sequence.</param>
+    /// <returns>The result task sequence.</returns>
+    /// <exception cref="T:ArgumentNullException">Thrown when the input task sequence is null.</exception>
+    /// <exception cref="T:ArgumentException">Thrown when index is below 0 or greater than source length.</exception>


below is atypical, unless it aligns with something I'd say less than?

you use the term negative elsewhere and that feels best?

bartelink · 2024-03-16T19:48:39Z

src/FSharp.Control.TaskSeq/TaskSeq.fsi

+    static member removeAt: index: int -> source: TaskSeq<'T> -> TaskSeq<'T>
+
+    /// <summary>
+    /// Return a new task sequence with the number of items starting at a given index removed.


Return a new task sequence with the number of items starting at a given index removed

Yields a new task sequence omitting the specified number of items at the given index

the removed is already in the name ? and it does not convey that there never was a materialized thing that things are being chopped out of; they're kinda just skipped. Maybe that's obvious, but I'm thinking of people that are hearing index and at index and thinking mentally of arrays. I guess I don't have beginner cred here so I don't know; i.e. it's just a suggestions/thought

Am also thinking that the term position might make sense somewhere, but you use index consistenly and I think its a good term

also instead of omitting, maybe dropping?

bartelink · 2024-03-16T19:55:58Z