overall path length is known
βββββ΄ββββββ
/foo/bar/baz
ββ¬β ββ¬β ββ¬β
p1 p2 p3 - p1, p2, p3 are separate writes; lengths unknown
If I understand it correctly, this is an argument for only trapping if the source collection isn't entirely consumed by the operation, allowing the target buffer to have a suffix that is untouched by the operation.
My motivation in recommending trapping if the source collection is too short was that otherwise callers would need to carefully remember to compare the returned index against target.endIndex
. This is true, but as your example shows, there is some real value in that:
let source: some Collection<some Collection<Foo>> = ...
let count = reduce(into: 0) { $0 += $1.count }
let target = UnsafeMutableBuffer<Foo>.allocate(capacity: count)
var remainder = target[...]
for component in source {
let end = remainder.initialize(fromElements: component) // traps if no space
remainder = remainder[end...]
}
precondition(remainder.isEmpty, "Underfilled buffer")
This is certainly quite a bit nicer (and much, much easier to get right) than the awkward code we currently have to write:
var start = target.startIndex
for component in source {
let remainder = UnsafeMutableBufferPointer(rebasing: target[start...])
var it: Component.Iterator // We can't even spell this with the `some Collection` syntax π©
(it, start) = remainder.initialize(from: component)
precondition(it.next() == nil, "Overflowing buffer")
}
precondition(start == target.endIndex, "Underfilled buffer")
Forgetting to check that it.next() == nil
is a very common issue, and so rolling that check directly into initialize(fromElements:)
seems very much desirable.
I retract my second suggestion -- initialize(fromElements:)
should only trap if the source collection is too large, not when it's too small. (This has the benefit of preserving the proposed API, just requiring a semantic change.)
(Note: While there is a similar argument for allowing copying only a prefix of the source buffer, that seems far less convincing to me -- primarily because (as we discussed) allowing that would require the method to return an index, which would complicate both the implementation and (more importantly) the API contract. Developers with such use cases will need to either fall back to the sequence-based APIs (which remain available), or properly slice their input collections.)
Forgetting to check that the returned value is the endIndex
of the target buffer is still going to be an issue, but as long as the function isn't marked @discardableResult
, at least there will be a warning if someone forgets to check it. (It helps that it's a simple value in this case, rather than a scary tuple.
In the slice case, there is a chance for confusing endIndex
with count
-- as in doing precondition(result == target.count)
rather than precondition(result == target.endIndex)
-- which might be worth calling out in the docs.