[swift-evolution] Stdlib closure argument labels and parameter names

Xiaodi Wu xiaodi.wu at gmail.com
Wed Jun 22 20:19:03 CDT 2016 

On Wed, Jun 22, 2016 at 8:12 PM, Sean Heber <sean at fifthace.com> wrote:

> How about:
>
> let results = possibilities.where(matching: closure)
>

My understanding is that we're restricting the bikeshedding here to the
closure label. Renaming `filter` is discussed in another thread, though
obviously that would change the name of the label as well.


>
> :)
>
> l8r
> Sean
>
> Sent from my iPad
>
> On Jun 22, 2016, at 8:00 PM, Xiaodi Wu via swift-evolution <
> swift-evolution at swift.org> wrote:
>
> filter(extractingWhere:)
> On Wed, Jun 22, 2016 at 18:53 Dave Abrahams via swift-evolution <
> swift-evolution at swift.org> wrote:
>
>>
>> on Wed Jun 22 2016, Xiaodi Wu <swift-evolution at swift.org> wrote:
>>
>> > I'll duly oblige with some pushback on `suchThat`. I get that you're
>> trying
>> > to clarify whether filter retains or gets rid of elements that match the
>> > predicate, but I don't think "filter such that" expresses this idea at
>> all.
>> >
>> > Comparing to "filter where," "filter such that" is equally susceptible
>> to
>> > misinterpretation that you are filtering to remove elements that are
>> > matched. For example: "find me some apples, filtering such that are
>> > bruised."
>>
>> Hahaha, that's a very different interpretation of “such” that I hadn't
>> considered!  OK, suppose it was “soEach:” ?
>>
>>     let primes = xs.filter(soEach: isPrime)
>>
>> > I'd suggest that if you want to be perfectly clear, you'd need something
>> > like `filter(keepingWhere:)`.
>>
>>     let primes = xs.filter(keepingWhere: isPrime)
>>
>> A slight problem is that filter is nonmutating, so all elements are
>> “kept.”  But maybe that's just Dave being overly concerned with unlikely
>> misinterpretations at the cost of “naturalness.”
>>
>> Further thoughts?
>>
>> > On Wed, Jun 22, 2016 at 18:33 Dave Abrahams via swift-evolution <
>> > swift-evolution at swift.org> wrote:
>> >
>> >>
>> >> on Tue Jun 21 2016, Dave Abrahams <swift-evolution at swift.org> wrote:
>> >>
>> >> > on Mon Jun 20 2016, Brent Royal-Gordon <swift-evolution at swift.org>
>> >> wrote:
>> >> >
>> >> >>> A couple of weeks ago we started to notice that we had some
>> >> poorly-named
>> >> >>> closure parameters and argument labels in the standard library, so
>> we
>> >> >>> did a complete audit of the standard library's APIs and came up
>> with a
>> >> >>> preliminary proposal for changes, which we applied in a branch and
>> you
>> >> >>> can review in https://github.com/apple/swift/pull/2981.  Let's
>> please
>> >> >>> carry on further discussion here rather than in the pull request,
>> >> though.
>> >> >>
>> >> >> In general, I like this; `orderingBy` is a particularly nice
>> >> >> improvement over the old `isOrderedBefore` convention.
>> >> >
>> >> > I don't really love the use of “by”, FWIW, but I thought
>> `orderingWith`
>> >> > was more confusable (ordering A with B might swap A and B, whereas
>> the
>> >> > parameter is a closure).  It could be argued, though, that I am being
>> >> > overly concerned with unlikely misinterpretations, at the cost of
>> >> > “naturalness”—a known weakness of mine ;-).  Anyway, as ever I'm
>> open to
>> >> > discussion on this.
>> >> >
>> >> >> A few specific comments about things I don't like:
>> >> >>
>> >> >> * In `map` and `flatMap`, I'm not sure how much `transform` buys us
>> >> >>   over `elementTransform`.
>> >> >
>> >> > I think you mean the converse.  And I agree that `elementTransform`
>> >> > is probably not an improvement over `transform`.
>> >>
>> >> ...and I've gone back to `transform` in my PR.
>> >>
>> >> >> * In general, I'm not a fan of most of the changes away from `where`
>> >> >> labels.
>> >> >
>> >> > The only such changes I can find are in
>> >> >
>> >>
>> https://github.com/apple/swift/pull/2981/commits/3418eede88d724ad23731fe8f412f51e03cf5106
>> >> >
>> >> > Note that part of this change was to make all filter closures
>> >> > consistent; in the main `filter` API there was no label at all.
>> >> > However, we felt that there's a real clarity problem with the
>> polarity
>> >> > of the argument (we talk about “filtering things out” but the closure
>> >> > indicates which elements to keep).  And we couldn't find a
>> “where”-based
>> >> > name that began to clarify it.
>> >> >
>> >> > I will argue that even changing to “suchThat,” as in the PR, does not
>> >> > sufficiently clarify the closure's polarity, and the only true fix
>> for
>> >> > filter is to use a different base name (some have suggested “select,”
>> >> > and I have other ideas), but that is out of scope for this particular
>> >> > set of changes.  So if the community is happier with a “where” label
>> >> > here I can live with it.  I do think “suchThat” is marginally
>> clearer.
>> >>
>> >> I have not received any further pushback on “suchThat,” so I've left it
>> >> alone.
>> >>
>> >> >
>> >> >> Those are a nice, straightforward convention applied broadly across
>> >> >> the Sequence APIs. (Yes, I criticized `where` as a method name in
>> >> >> another thread, but I don't think `where` is a problem when there's
>> a
>> >> >> function base name to give it context.) When they don't work, that's
>> >> >> usually because of a less-than-ideal base name. I'm not saying that
>> >> >> *all* base names that aren't compatible with `where` should be
>> >> >> changed, but rather that if `where` is not enough, that's an API
>> >> >> smell.
>> >> >>
>> >> >> * In particular, `elementWhere` is not a good label for the same
>> >> >> reason that `removeElement` is not a good name. Session 403 last
>> week
>> >> >> actually talked about this between roughly minutes 8 and 11. (I'm
>> sure
>> >> >> you know about its content; you probably saw it before we did.)
>> >> >
>> >> > Yes I do, and I think you misinterpreted the message in that session.
>> >> > There's nothing wrong with repeating type information when it's
>> >> > necessary for clarity or fluency at the use-site.  In the case of
>> >> > `contains(elementWhere:)`, it's there for fluency:
>> >> >
>> >> >        customers.contains(where: isSingle)
>> >> >
>> >> > doesn't read as well as:
>> >> >
>> >> >        customers.contains(elementWhere: isSingle)
>> >> >
>> >> > The point is not to imagine that every argument should be preceded by
>> >> > a noun, and repetition of type information is often the result of
>> >> > trying to do that.
>> >> >
>> >> >> * I like `separatedWhere` on `split`, but I think the Equatable
>> >> >> version needs a similar renaming.
>> >> >
>> >> > That's a nice thought; I think it's arguably out-of-scope here,
>> though.
>> >> >
>> >> >> Perhaps `separatedBy`?  `separatedOn`? The usual opposite of
>> `where`,
>> >> >> `of`, doesn't work here. (Alternatively, `separatedWhere` could be
>> >> >> `separatorWhere` instead, but that's not quite as elegant.)
>> >> >
>> >> > I'd want to consider variations of `separatingAt` or `onSeparator` or
>> >> > `atSeparator` too... which makes me thing “separatedWhere” might not
>> be
>> >> > as good as “separatingWhere” for the closure version.
>> >> >
>> >> >> * I'm very uncomfortable with the amount of weight
>> >> >> `accumulatingResultBy` adds to `reduce`. `combinedBy` seems
>> perfectly
>> >> >> cromulent to me. I'm even more concerned by your suggestion in the
>> >> >> pull request body of
>> >> >> `accumulating(startingFrom:combiningBy:)`. `reduce` is a subtle and
>> >> >> slightly confusing operation; adding more words to its call sites
>> will
>> >> >> not solve that problem. If you want to invent a new name from whole
>> >> >> cloth, I would probably use something like `combining(with
>> >> >> initialResult: T, by nextResult: (T, Element) -> T)`. (For that
>> >> >> matter, while we're working in this area, `sequence(first:next:)`
>> >> >> could use a similar coat of paint.)
>> >> >
>> >> > As with `filter(suchThat:`, `reduce(accumulatingResultBy:` is
>> attempting
>> >> > to solve with an argument label what IMO is a grave weakness in
>> clarity
>> >> > of the base name.  If you read the documentation for `reduce`, you'll
>> >> > see that it's all about accumulating a result, and if you consider
>> that
>> >> > its current signature often leads to O(N^2) behavior and we are
>> thinking
>> >> > about adding an overload that takes its “accumulator” inout, the
>> >> > arguments for avoiding the name “accumulate” get progressively
>> weaker.
>> >> > But as noted earlier, changing base names is out-of-scope for this
>> >> > proposal.  As with “filter,” I could live with leaving this alone,
>> >> > though I do believe “accumulatingResultBy:” is a real improvement in
>> >> > clarity.
>> >>
>> >> ...but I think it's overly specific at the expense of smoothness.  So
>> >> I've removed `Result` from that name.
>> >>
>> >> >> * I agree with the comment on GitHub that `invoke` should be
>> >> >> `execute`.
>> >> >
>> >> > Why?  Rationales help.
>> >> >
>> >> >> If you see a distinction between the two cases on the number of
>> >> >> arguments, I would then suggest `passTo` as the label on these
>> >> >> methods: `views.forEach(passTo: addSubview)`,
>> >> >> `withUnsafeBufferPointer(&bytes, passTo: Data.init(buffer:))`.
>> >> >
>> >> > Those are intriguing ideas, but that direction tends to suggest this
>> >> > would be better:
>> >> >
>> >> >   views.passEach(to: addSubview)
>> >> >   passUnsafeBufferPointer(to: Data.init(buffer:))
>> >> >
>> >> > ...until you pass a trailing closure:
>> >> >
>> >> >   views.passEach { addSubView($0) }
>> >> >   passUnsafeBufferPointer { Data.init(buffer:$0) }
>> >> >
>> >> > (note: withUnsafeBufferPointer takes only one argument, a closure).
>> >> >
>> >> >>
>> >> >> * It's a little odd that you're using `comparingBy` for `Equatable`
>> >> >> and `orderingBy` for `Comparable`. Did you judge `equatingBy` to be
>> >> >> too awkward?
>> >> >
>> >> > Yes, and because it's not “equating,” which would mean using equality
>> >> > (==) it's “testing equivalence” with respect to the predicate.
>> >> >
>> >> >> Perhaps the real problem is that `Equatable` ought to be
>> `Comparable`
>> >> >> and `Comparable` ought to be `Orderable`?
>> >> >
>> >> > I don't think so, personally, but regardless I consider such a change
>> >> > out-of-scope for this proposal.
>> >> >
>> >> >> Or maybe `comparingBy` should just be something more general, like
>> >> >> `matchingBy`? That would make perfectly sensible but slightly odd
>> use
>> >> >> cases like this one read better:
>> >> >>
>> >> >>      let isAnIdiot = luggageCombination.starts(with: [1, 2, 3, 4,
>> >> >> 5], matchingBy: <=) // Matches [1,2,3,4,5], but also [1,1,1,1,1],
>> >> >> [1,2,3,2,1], etc.
>> >> >
>> >> > That would not be legal, as <= is not an equivalence relation.  You
>> >> > could think about redefining the meaning of `starts(with:` to not
>> >> > require an equivalence relation, but that's something I'm not
>> confident
>> >> > *I* know how to do meaningfully, and regardless is again
>> out-of-scope.
>> >> >
>> >> >> Very soon (hopefully), I will be posting an early draft of a
>> proposal
>> >> >> renaming the various first/last/prefix/suffix/etc. APIs. I believe
>> the
>> >> >> only place it touches on your proposal is in
>> >> >> `starts(with:isEquivalent:)`, but I think your changes to the second
>> >> >> parameter label can be easily incorporated into what I'm doing.
>> >> >
>> >> > Great!
>> >>
>> >> I'm going to write up the proposal ASAP based on the current PR unless
>> I
>> >> get more feedback.
>> >>
>> >> Thanks,
>> >>
>> >> --
>> >> Dave
>> >>
>> >> _______________________________________________
>> >> swift-evolution mailing list
>> >> swift-evolution at swift.org
>> >> https://lists.swift.org/mailman/listinfo/swift-evolution
>> >>
>> > _______________________________________________
>> > swift-evolution mailing list
>> > swift-evolution at swift.org
>> > https://lists.swift.org/mailman/listinfo/swift-evolution
>> >
>>
>> --
>> Dave
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
> _______________________________________________
> 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/20160622/2fa06bf9/attachment.html>

More information about the swift-evolution mailing list