Copy #52

CTMacUser · 2020-12-22T04:05:56Z

Description

This change adds methods that introduces new elements to a collection via overwriting existing elements (instead of inserting more space).

Detailed Design

The are methods that copy across subsequences of the same collection, both in the forward and backward directions. The main versions use a separate object as the source. There are combinations for Sequence vs. Collection source, and copying over the prefix vs. suffix. There is another main version that copies suffix to suffix.

extension MutableCollection {
    /// Copies the prefix of the given sequence on top of the prefix of this collection.
    public mutating func copy<S>(from source: S) -> (copyEnd: Index, sourceTail: S.Iterator) where S : Sequence, Self.Element == S.Element

    /// Copies the prefix of the given collection on top of the prefix of this collection.
    public mutating func copy<C>(collection: C) -> (copyEnd: Index, sourceTailStart: C.Index) where C : Collection, Self.Element == C.Element

    /// Copies, in forward traversal, the prefix of a subsequence on top of the prefix of another, using the given bounds to demarcate the subsequences.
    public mutating func copy<R, S>(forwardsFrom source: R, to destination: S) -> (sourceRead: Range<Index>, destinationWritten: Range<Index>) where R : RangeExpression, S : RangeExpression, Self.Index == R.Bound, R.Bound == S.Bound
}

extension MutableCollection where Self : BidirectionalCollection {
    /// Copies the prefix of the given sequence on top of the suffix of this collection.
    public mutating func copy<S>(asSuffix source: S) -> (copyStart: Index, sourceTail: S.Iterator) where S : Sequence, Self.Element == S.Element

    /// Copies the prefix of the given collection on top of the suffix of this collection.
    public mutating func copy<C>(collectionAsSuffix source: C) -> (copyStart: Index, sourceTailStart: C.Index) where C : Collection, Self.Element == C.Element

    /// Copies the suffix of the given collection on top of the suffix of this collection.
    public mutating func copy<C>(backwards source: C) -> (writtenStart: Index, readStart: C.Index) where C : BidirectionalCollection, Self.Element == C.Element

    /// Copies, in reverse traversal, the suffix of a subsequence on top of the suffix of another, using the given bounds to demarcate the subsequences.
    public mutating func copy<R, S>(backwardsFrom source: R, to destination: S) -> (sourceRead: Range<Index>, destinationWritten: Range<Index>) where R : RangeExpression, S : RangeExpression, Self.Index == R.Bound, R.Bound == S.Bound
}

Documentation Plan

The methods have documentation comments. There is also a guide file.

Test Plan

There is a XC Test file for these methods.

Source Impact

The change adds API.

Checklist

I've added at least one test that validates that my change is working, if appropriate
I've followed the code style of the rest of the project
I've read the Contribution Guidelines
I've updated the documentation if necessary

Add method to element-mutable collections that takes a sequence of the same element type to read its elements and assign them on top of the collection's current elements. The copying stops upon reaching the end of the shorter sequence. The method returns two items. The first is one index past the last destination element written over. The second is an iterator for the elements of the source sequence that were not used for copying.

Add method to element-mutable collections that takes a collection of the same element type to read its elements and assign them on top of the receiver's current elements. The copying stops upon reaching the end of the shorter collection. The method returns two items. The first is one index past the last destination element written over. The second is one index past the last source element read from.

Add method to element-mutable bidirectional collections that takes a sequence of the same element type to read its elements and assign them on top of the collection's last elements. The copying stops upon reaching the end of the source sequence or the beginning of the receiver, whichever is hit first. The method returns two items. The first is the index for the first destination element written over. The second is an iterator for the elements of the source sequence that were not used for copying. Add another method that does the same thing but the source parameter is instead also a collection; its second returned item is instead one index past the last source element read from. For the tests over the copying methods where both the receiver and argument are collections, use the returned index bounds to compare equality for the targeted subsequences.

Add method to element-mutable bidirectional collections that takes a bidirectional collection with the same element type to read its last elements and assign them on top of the receiver's last elements. The copying is done backwards and stops upon at least one of the collections reaching their beginning (whichever hits it first). The method returns two items: the starting indices of each collection's suffix that participated in the copying.

Add method to element-mutable collections that takes range expressions for two subsequences of the receiver and copies elements from one subsequence on top of the elements for the other. The copying is done forwards, targeting the subsequences' prefixes until at least one subsequence runs out of elements. Add an overload for bidirectional collections that does its copies in reverse over the subsequences' suffixes instead. The methods return two items; range expressions bounding the parts of the subsequences that actually participated in copying.

Improve the summary descriptions for the methods of element-mutable collections that copy over elements, but only for the overloads where the source is a sequence or collection. They clarify what parts of the receiver and source are touched. Rename the "copyOntoSuffix" methods to use "copy" by itself as the base names, and use new parameter labels to differentiate the overloads. Now all of the element-copying methods use the same base name.

mdznr

Naming
I wouldn’t have guessed this would be the behavior of a function named copy. This may be because I’ve programmed a lot more Objective-C than C++ recently. I suspect many Swift programmers are also in that same situation.

Perhaps the naming should include some clarifying words, like “prefix”, or “over”/“overwrite”?

overwritePrefix(with:)?
Sequence and Collection
I was a bit confused as to why there is a copy(from:) taking a Sequence and a differently named copy(collection:) (omitting the “from” terminology) for Collection.

If I’m working with two Arrays, it’s not clear which function I should use and why.
```
var destination = ["A", "B", "C", "D", "E", "F"]
let source = ["a", "b", "c"]
destination.copy(from: source)
destination.copy(collection: source)
```
Are these both necessary? And if so, how can it be made more clear which function to use?
Return Value
Should these functions be marked as @discardableResult? Seems like it’s valid to call these functions without making use of the return value.
Performance
There are a few shortcuts that could probably made depending on the input sizes. As one example, if the source is larger than the destination, why bother going through the items one-by-one when a prefix of self.count can be taken from the source?

Minor:

I’ve noticed that the rest of this repository uses a single between sentences, but these changes include comments and documentation using two. This should be consistent.

CTMacUser · 2020-12-24T16:29:35Z

Naming
I wouldn’t have guessed this would be the behavior of a function named copy. This may be because I’ve programmed a lot more Objective-C than C++ recently. I suspect many Swift programmers are also in that same situation.

Perhaps the naming should include some clarifying words, like “prefix”, or “over”/“overwrite”?

overwritePrefix(with:)?

Due to the nature of C++'s output iterator concept, its copying includes both memory overwriting and new-space insertion! Swift puts those as two separate collection concepts (MutableCollection vs. RangeReplaceableCollection). RRC already covers copying that inserts new memory spaces, but they don't use "copy" in their names (replaceSubrange, append, and insert), so "copy" was free to use. Maybe "overwrite" or similar would be better.

Sequence and Collection
I was a bit confused as to why there is a copy(from:) taking a Sequence and a differently named copy(collection:) (omitting the “from” terminology) for Collection.

If I’m working with two Arrays, it’s not clear which function I should use and why.
var destination = ["A", "B", "C", "D", "E", "F"]
let source = ["a", "b", "c"]
destination.copy(from: source)
destination.copy(collection: source)
Are these both necessary? And if so, how can it be made more clear which function to use?

The sequence vs. collection methods differ in their return type. Since a Sequence may be single-pass, I can only return an Iterator to the unused elements. For a Collection, I return an Index to the first unused element (or endIndex if all were used). The interfaces for the second member of the return value are obviously incompatible, so I can't/shouldn't reuse a (full) name for both.

Hmm, would a better option than a Sequence be an inout IteratorProtocol parameter instead? But that would put the extra step of converting onto the user. (A similar thing happened when I proposed an initializer to CollectionOfOne that reads the first element from a sequence/iterator. Maybe the same solution will work here.)

Return Value
Should these functions be marked as @discardableResult? Seems like it’s valid to call these functions without making use of the return value.

I wonder about that, too. Since it's (currently) non-discardable, the user has to think about what if one of the collections/sequences wasn't completely used. If the user knows that the source and destination are the same length, or otherwise doesn't care, they can use the "_ =" syntax.

Performance
There are a few shortcuts that could probably made depending on the input sizes. As one example, if the source is larger than the destination, why bother going through the items one-by-one when a prefix of self.count can be taken from the source?

Are you implying the existence of a procedure that can do an O(1) bulk overwriting-copy? Otherwise, this wouldn't save any time. And if there is a (refined) protocol that can guarantee this (and also conform to RandomAccessCollection so self.count would be O(1)), that wouldn't help with any other sequence.

Minor:

I’ve noticed that the rest of this repository uses a single between sentences, but these changes include comments and documentation using two. This should be consistent.

I'll check that out.

Trim down the summary descriptions of this library to minimize (or remove) descriptions of the non-main methods. Ensure the main documentation file ends with a line-breaking sequence.

Add method to element-mutable collections that takes an iterator of the same element type to extract enough of its elements for assigning them on top of the receiver's leading elements (in order). The iterator argument is taken as an in-out reference, so its post-call state only points to its unread elements. Copying during the call stops upon either the iterator running out of elements or the receiver running out of untouched elements. The method returns the index to the first element after the overwritten elements, or the past-the-end index if all elements of the receiver were overwritten. For element-mutable collections that also support bidirectional traversal, add another method that takes an iterator in order to extract its elements to copy over existing elements of the receiver, but targets the receiver's trailing elements instead. Elements in the overwritten suffix maintain the same order they had in the source iterator. This method returns the index of the first element that was overwritten, or the past-the-end index if no element of the receiver was touched.

Move the secret implementation methods to the end of their file. Change the methods that copy from a sequence to return only the index of the first untouched element of the receiver. (Use the iterator-based overload to track unread elements from the source.) Move the location of the revised tests in their file. Update the documentation, including the prime example.

For the secret implementation to copy from an iterator to the suffix of a collection, add a parameter controlling which direction copying occurs. Now the replaced elements may stay in their original order, or be stored in reverse.

Change the name, return order, and implementation of the method of element-mutable collections that can replace their prefix with elements from a collection. Do the same thing for the method that copies across collection suffixes backwards, while removing the method that copies a collection's prefix to another's suffix.

Acknowledge that the base name of the methods was changed, and give reason for the change.

CTMacUser · 2020-12-31T07:33:17Z

I renamed the methods from "copy" to "overwrite" and did a lot of the other changes suggested by @mdznr. Check it out.

LucianoPAlmeida · 2021-01-02T02:25:34Z

Guides/Overwrite.md

+New methods are added to element-mutable collections:
+
+```swift
+extension MutableCollection {


This indeed seems a nice proposal and well implemented, but I was wondering that this functionality is kind of already covered in a more general way by RangeReplaceableCollection's replaceSubrange(:with:).
So those are more questions/suggestions:
What are the advantages of having this API over the existing one?
And also, related to naming, could this complement the existing one by being called replacePrefix(with:)?
I think this is more align with the convention and easier to reason about given the existing API's available.

RangeReplaceableCollection and MutableCollection generally can’t be substituted for each other; the API you mentioned is pretty much their sole spiritual overlap, and it wouldn’t work with collections that implement only MC.

Guides/Overwrite.md

Co-authored-by: Luciano Almeida <[email protected]>

adincebic · 2021-12-02T08:25:18Z

@swift-ci Please test

CTMacUser added 8 commits November 27, 2020 11:38

Correct name of parameter in documentation

8e19993

Correct names of tests

440456f

mdznr reviewed Dec 22, 2020

View reviewed changes

CTMacUser added 8 commits December 24, 2020 21:44

Rename the "copy" methods to "overwrite"

b354606

Redo documentation

3a9f943

Trim down the summary descriptions of this library to minimize (or remove) descriptions of the non-main methods. Ensure the main documentation file ends with a line-breaking sequence.

Add direction parameter when copying from an iterator

788e7a8

For the secret implementation to copy from an iterator to the suffix of a collection, add a parameter controlling which direction copying occurs. Now the replaced elements may stay in their original order, or be stored in reverse.

Update documentation

50f5855

Acknowledge that the base name of the methods was changed, and give reason for the change.

Rename the files from "Copy" to "Overwrite"

79af788

LucianoPAlmeida reviewed Jan 2, 2021

View reviewed changes

Guides/Overwrite.md Outdated Show resolved Hide resolved

Correct spelling error

99d436b

Co-authored-by: Luciano Almeida <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copy #52

Copy #52

CTMacUser commented Dec 22, 2020

mdznr left a comment

CTMacUser commented Dec 24, 2020

CTMacUser commented Dec 31, 2020

LucianoPAlmeida Jan 2, 2021 •

edited

Loading

CTMacUser Jan 11, 2021

adincebic commented Dec 2, 2021

Copy #52

Are you sure you want to change the base?

Copy #52

Conversation

CTMacUser commented Dec 22, 2020

Description

Detailed Design

Documentation Plan

Test Plan

Source Impact

Checklist

mdznr left a comment

Choose a reason for hiding this comment

CTMacUser commented Dec 24, 2020

CTMacUser commented Dec 31, 2020

LucianoPAlmeida Jan 2, 2021 • edited Loading

Choose a reason for hiding this comment

CTMacUser Jan 11, 2021

Choose a reason for hiding this comment

adincebic commented Dec 2, 2021

LucianoPAlmeida Jan 2, 2021 •

edited

Loading