[swift-evolution] [SHORT Review] SE-0132: Rationalizing Sequence end-operation names

Xiaodi Wu xiaodi.wu at gmail.com
Mon Jul 25 23:50:48 CDT 2016


On Mon, Jul 25, 2016 at 8:35 PM, Dave Abrahams via swift-evolution <
swift-evolution at swift.org> wrote:

>
> 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)
>

+1 to this.


> 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.
>

I'm not enamored of this suggestion. It succeeds in reducing API surface
area, but at a severe cost to readability. You'd replace an unambiguous
phrase (removing prefix or suffix), the meaning of which is further
clarified by the consistent usage proposed in SE-0132, with a wordless
spelling using some combination of [$+.<]. Cognitively, also, it
substantially increases the burden for the reader: it replaces a single
argument with a nested series of arguments; first, one must understand the
meaning of the $ placeholder, then one must consider an addition or
subtraction operation, then the formation of a range, and in the last
example, the use of that range as a subscript argument--again, all
wordlessly. Finally, subscripts have so far not been "forgiving," while
today's `dropLast` very much is; this suggestion would add inconsistency by
using a subscript for a forgiving or "lenient" operation.


>
>   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.
>

Pyry's comments about the precedence issues with unary range operators are,
I think, serious enough that the unary operators should be reconsidered.
This suggestion might work but I wonder if we could do better than $.


> > ### 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
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160725/45ff1163/attachment.html>


More information about the swift-evolution mailing list