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

Dave Abrahams dabrahams at apple.com
Sat Jun 25 16:38:11 CDT 2016 

on Wed Jun 22 2016, Xiaodi Wu <xiaodi.wu-AT-gmail.com> wrote:

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

Correct, thanks.

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

-- 
-Dave

More information about the swift-evolution mailing list