[swift-evolution] [SHORT Review] SE-0132: Rationalizing Sequence end-operation names
Dave Abrahams
dabrahams at apple.com
Mon Jul 25 20:35:18 CDT 2016
on Sun Jul 24 2016, Chris Lattner <swift-evolution at swift.org> wrote:
> Hello Swift community,
>
> The review of "SE-0132: Rationalizing Sequence end-operation names"
> begins now and runs through July 26. Apologies for the short review
> cycle, but we’re right up against the end of source breaking changes
> for Swift 3. The proposal is available here:
>
> https://github.com/apple/swift-evolution/blob/master/proposals/0132-sequence-end-ops.md
>
> Reviews are an important part of the Swift evolution process. All
> reviews should be sent to the swift-evolution mailing list at
>
> https://lists.swift.org/mailman/listinfo/swift-evolution
>
> or, if you would like to keep your feedback private, directly to the review manager.
>
> What goes into a review?
>
> The goal of the review process is to improve the proposal under review
> through constructive criticism and contribute to the direction of
> Swift. When writing your review, here are some questions you might
> want to answer in your review:
>
> * What is your evaluation of the proposal?
I'm mostly very much in favor of this proposal, but I have some
thoughts.
First, though, I have to apologize for those wide tables, since I'm
listed as a co-author (because of a small design contribution). The
only way I've been able to read them is by looking at the markdown
source, so that's how I'm going to quote it here.
[Note to future authors: if you need to include a table, this is how you
can make it narrow enough:
https://github.com/apple/swift-evolution/blob/master/proposals/0118-closure-parameter-names-and-labels.md#proposed-solution.
The source is awful to read but it renders OK.]
> ## Proposed solution
>
> We sever the index-taking APIs from the others, forming two separate
> families, which we will call the "Sequence-end operations" and the
> "index-based operations". We then consider and redesign them along
> separate lines.
>
> ### Sequence-end operations
>
> Each of these APIs should be renamed to use a directional word based on
> its row in the table:
>
> | Operand | Directional word |
> | -------------------------------- | ------------------ |
> | **Fixed Size** |
> | First 1 | first |
> | Last 1 | last |
> | First (n: Int) | prefix |
> | ...with closure | prefix |
> | Last (n: Int) | suffix |
> | ...with closure | suffix |
> | **Searching From End** |
> | First matching element | first |
> | ...with closure | first |
> | Last matching element | last |
> | ...with closure | last |
>
> To accomplish this, `starts(with:)` should be renamed to
> `hasPrefix(_:)`, and other APIs should have directional words replaced
> or added as appropriate.
>
> Additionally, the word `drop` in the "Exclude" APIs should be replaced
> with `removing`. These operations omit the same elements which the
> `remove` operations delete, so even though the types are not always
> the same (`removing` returns `SubSequence`, not `Self`), we think they
> are similar enough to deserve to be treated as nonmutating forms.
Unfortunately there's a semantic difference that I hadn't noticed
before: the mutating “remove” operations have a precondition that there
be at least as many elements as are being removed. “Drop,” like “pop,”
is forgiving of such overruns. I think this is solvable; my suggestion
is below
> These changes yield (altered names **bold**):
>
> | | Get | Index | Exclude | Remove (1) | Pop (1) | Equate (2) |
> | -------------------------------- | ----------------------------- | ------------------------------------- | ----------------------------------------- | --------------------- | ------------ | ------------------------------------------ |
> | **Fixed Size** |
> | First 1 | C.first | - | **S.removingFirst()** | C.removeFirst() | C.popFirst() | - |
> | Last 1 | C.last | - | **S.removingLast()** | C.removeLast() | C.popLast() | - |
> | First (n: Int) | S.prefix(3) | - | **S.removingPrefix(3)** | **C.removePrefix(3)** | - | **S.hasPrefix([x,y,z])** |
> | ...with closure | S.prefix(while: isPrime) | - | **S.removingPrefix(while: isPrime)** | - | - | **S.hasPrefix([x,y,z], by: ==)** |
> | Last (n: Int) | S.suffix(3) | - | **S.removingSuffix(3)** | **C.removeSuffix(3)** | - | - |
> | ...with closure | - | - | - | - | - | - |
> | **Searching From End** |
> | First matching element | - | **C.firstIndex(of: x)** | - | - | - | - |
> | ...with closure | S.first(where: isPrime) | **C.firstIndex(where: isPrime)** | - | - | - | - |
> | Last matching element | - | - | - | - | - | - |
> | ...with closure | - | - | - | - | - | - |
My suggestion would be to make the remove()
operations more forgiving:
rename popFirst() to removeFirst()
rename popLast() to removeLast()
kill removeFirst(n)
kill removeLast(n)
The “forgiving” forms of x.removeFirst(n) and x.removeLast(n) can be
expressed as:
let i = x.index(x.startIndex, offsetBy: n, limitedBy: x.endIndex)
x.removeSubrange(..<i)
let i = x.index(x.endIndexIndex, offsetBy: -n, limitedBy: x.startIndex)
x.removeSubrange(i..<)
I realize that's quite verbose. We could of course just make
removePrefix(n) and removeSuffix(n) forgiving, but I have long believed
that the “prefix/suffix” methods should go one of two ways:
a. they get a label that clarifies the meaning of the argument, e.g.
x.removePrefix(ofMaxLength: n)
x.removeSuffix(ofMaxLength: n)
b. they are given a recognizable domain-specific notation such as:
x.removeSubrange($+n..<)
x.removeSubrange(..<$-n)
I am strongly in favor of this answer (which is implementable within
the framework of this proposal) because of the way it reduces API
surface area and leverages the user's understanding of how ranges
work.
It also implies we can replace
x.removingPrefix(n)
x.removingSuffix(n)
with
x[$+n..<]
x[..<$-n]
for Collections.
That would admittedly leave single-pass Sequences without an API for
dropping the first N elements. I am inclined to think that interface
should be moved to Iterator.
The introduction of such notation raises the question of whether we
need unary range operators, and could instead go with
x[i..<$] and x[$..<i]
which is after all only one character longer than
x[i..<] and x[..<i]
and stays out of the territory of the prefix/suffix “pack/unpack”
operators that are likely to be used for generic variadics.
> ### Index-based operations
>
> Because these APIs look up elements based on their indices, we believe
> these operations should be exposed as subscripts, and ideally should
> look like other slicing operations:
>
> ```swift
> let head = people[..<i]
> let tail = people[i..<]
> let rearrangedPeople = tail + head
> ```
>
> <!-- Comment to make my editor happy -->
>
> We will accomplish this by introducing two new types, `IncompleteRange`
> and `IncompleteClosedRange`. These are similar to `Range` and
> `ClosedRange`, except that the bounds are optional.
>
> To construct them, we will introduce both prefix and suffix operators
> taking a non-optional bound, and infix operators taking optional bounds.
> (We offer both because `c[..<i]` is more convenient than `c[nil ..< i]`,
> but doesn't allow you to dynamically choose between supplying and
> omitting a bound.) These will follow the existing convention: `..<` will
> construct the half-open `IncompleteRange`, while `...` will construct
> `IncompleteClosedRange`.
I believe the `$+n..<i` idea is still implementable with these basic
types, just with an enum instead of optionals. I'll take a shot at it
tonight if I can get a few minutes.
> Rather than continuing to proliferate overloads of slicing subscripts,
> we will also introduce a new `RangeExpression` protocol which allows
> any range-like type to convert itself into a plain `Range<Index>`
> appropriate to the collection in question. Thus, there should only be
> two range subscripts: one taking `Range<Index>`, and one taking
> everything else.
>
> We will also modify the existing `removeSubrange(_:)` and
> `replaceSubrange(_:with:)` calls to take `RangeExpression` instances,
> thereby merging many existing variants into one while simultaneously
> extending them to support `IncompleteRange` and `IncompleteClosedRange`.
> Though technically additive, we believe this is an easy win.
>
> Thus, the table above becomes:
>
> | | Type | Get | Remove | Replace |
> | ------------------------------------------------ | --------------------------------- | -------------------- | ----------------------------------- | ----------------------------------------------------------- |
> | **Based on Index, Arbitrary** |
> | (i: Index) ..< (j: Index) | Range\<Index> | C[i ..< j] | C.removeSubrange(i ..< j) | C.replaceSubrange(i ..< j, with: [x,y]) |
> | ...Countable | CountableRange\<Index> | C[i ..< j] | C.removeSubrange(i ..< j) | C.replaceSubrange(i ..< j, with: [x,y]) |
> | (i: Index) ... (j: Index) | ClosedRange\<Index> | C[i ... j] | C.removeSubrange(i ... j) | C.replaceSubrange(i ... j, with: [x,y]) |
> | ...Countable | CountableClosedRange\<Index> | C[i ... j] | C.removeSubrange(i ... j) | C.replaceSubrange(i ... j, with: [x,y]) |
> | **Based on Index, From End** |
> | startIndex ..< (i: Index) | **IncompleteRange\<Index>** | **C[..\<i]** | **C.removeSubrange(..\<i)** | **C.replaceSubrange(..\<i, with: [x,y])** |
> | (i: Index) ..< endIndex | **IncompleteRange\<Index>** | **C[i..\<]** | **C.removeSubrange(i..\<)** | **C.replaceSubrange(i..\<, with: [x,y])** |
> | startIndex ... (i: Index) | **IncompleteClosedRange\<Index>** | **C[...i]** | **C.removeSubrange(...i)** | **C.replaceSubrange(...i, with: [x,y])** |
>
> However, it should be implemented with merely:
>
> | | Type | Get | Remove | Replace |
> | --------------------------------------------- | --------------------------------------------------- | -------------------- | ----------------------------------- | ----------------------------------------------------------- |
> | (i: Index) ..< (j: Index) | Range\<Index> | C[i ..< j] | C.removeSubrange(i ..< j) | C.replaceSubrange(i ..< j, with: [x,y]) |
> | Everything else | RangeExpression where Bound == Index | C[i ... j] | C.removeSubrange(i ... j) | C.replaceSubrange(i ... j, with: [x,y]) |
>
> ## Detailed design
>
> ### Sequence-end operations
>
> The following methods should be renamed as follows wherever they appear
> in the standard library. These are simple textual substitutions; we
> propose no changes whatsoever to types, parameter interpretations, or
> other semantics.
>
> | Old method | New method |
> | ------------------------------------------------- | ------------------------------------------------------- |
> | `dropFirst() -> SubSequence` | `removingFirst() -> SubSequence` |
> | `dropLast() -> SubSequence` | `removingLast() -> SubSequence` |
> | `dropFirst(_ n: Int) -> SubSequence` | `removingPrefix(_ n: Int) -> SubSequence` |
> | `drop(@noescape while predicate: (Iterator.Element) throws -> Bool) rethrows -> SubSequence` | `removingPrefix(@noescape while predicate: (Iterator.Element) throws -> Bool) rethrows -> SubSequence` |
I'm concerned with how the above fits into the scheme. Writing it out
is:
x[(x.firstIndex(where: {!predicate($0)}) ?? x.endIndex)..<$]
and that's just for Collections. Drat; we might not be able to get rid
of these “removingPrefix” operations altogether. OK, I'm out of time so
I'll have to get back to this.
--
Dave
More information about the swift-evolution
mailing list