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

[Erica Sadun]

On Jun 25, 2016, at 4:25 PM, Dave Abrahams <dabrahams at apple.com> wrote:
> on Wed Jun 22 2016, Erica Sadun <erica-AT-ericasadun.com> wrote:
> On Jun 20, 2016, at 3:25 PM, Dave Abrahams via swift-evolution <swift-evolution at swift.org> wrote:
>> 
>> -    func forEach<S: SequenceType>(_ body: (S.Iterator.Element) -> ())
>> +    func forEach<S: SequenceType>(invoke body: (S.Iterator.Element) -> ())
>> 
>> Adding an external label makes sense here. This is a procedural call and
>> using it within the parens should have a "code ripple".
> 
> I don't think I understand what you mean here.

When using a procedural "trailable" closure inside parentheses, the intention
to do so should be clear:

p(x, perform: {...})
p(x, do: {...})

vs

p(x) {
   ...
}

Anyone reading this code can immediately identify that an otherwise trailing
closure has been pulled inside the signature because the call has become 
significantly more  complex. The point-of-coding decision ripples through
to point-of-reading/point-of-maintenance.

>> That said, would prefer `do` or `perform` over `invoke` or `invoking` as in
>> `runRaceTest`, `_forAllPermutationsImpl`, `expectFailure`, etc. 
> 
> Sorry, again, I'm a little lost.  Forgive my overly-literal brain but
> could you please spell out how those latter 3 names relate to the choice
> of `do` or `perform` over `invoke`?   

I read through the pull request. I grouped related modifications
together, although not exhaustively. They fall under the same umbrella, 
choosing `do` or `perform` compared to `invoke` or `invoking`.

-    _forAllPermutationsImpl(index + 1, size, &perm, &visited, body)
+    _forAllPermutationsImpl(index + 1, size, &perm, &visited, invoke: body)

-public func runRaceTest(trials: Int, threads: Int? = nil, body: () -> ()) {
+public func runRaceTest(
+  trials: Int, threads: Int? = nil, invoking body: () -> ()
+) {

-public func expectFailure(${TRACE}, body: () -> Void) {
+public func expectFailure(${TRACE}, invoking body: () -> Void) {


> 
>> This also applies where there's a `body` label instead of an empty
>> external label.
> 
> We don't have any methods with a `body` label IIUC.

You did, as in the examples just above, which you recommend a rename to `invoke` or
`invoking`.

>> -public func withInvalidOrderings(_ body: ((Int, Int) -> Bool) -> Void) {
>> +public func withInvalidOrderings(invoke body: ((Int, Int) -> Bool) -> Void) {
>> 
>> For any with/external label pair, I'd prefer `with-do` or `with-perform` 
>> over `with-invoke`.
> 
> OK.  Is there a rationale, or is it just personal taste?

Strong personal taste, backed by tech writing. Simpler words. 
From the Corpus of Contemporary American English:
do: 18
perform: 955
invoke: does not appear on list.
> 
>> -  return IteratorSequence(it).reduce(initial, combine: f)
>> +  return IteratorSequence(it).reduce(initial, accumulatingBy: f)
>> 
>> For `reduce`, I'd prefer `applying:` or `byApplying:`
> 
> Makes sense.
> 
>> Similarly in `starts(with:comparingBy:)`, I'd prefer byComparing`,
> 
> I don't see how that works.  You're not comparing the closure with
> anything.

I get your point but I think it's unnecessarily fussy. 

Alternatives are slim on the ground. `usingComparison:` is too long,
as is `byComparingWith:` (which still reads better but you will point
out can be mistaken by some pedant to mean that the sequence is being
compared to the closure), and you won't allow for `comparing:`.
I'm not in love with `comparingWith:` but it reads slightly better to me 
than `comparingBy:`.


> 
>> min/max, byOrdering
> 
> Likewise, you're not ordering the closure.

Same reasoning.

> 
>> -      ).encode(encoding, output: output)
>> +      ).encode(encoding, sendingOutputTo: processCodeUnit)
>> 
>> How about `exportingTo`?
> 
> “export” is freighted with various connotations that I don't think we
> want to make here.  In fact it doesn't mean anything more exotic than
> “sending output to.”

For a language that treasures concision and clarity, this may be clear
but it's notably inconcise. (Yes, that is a new word.)

> 
>> -  tempwords.sort(isOrderedBefore: <)
>> +  tempwords.sort(orderingBy: <)
>> 
>> With `sort` and `sorted`, I'd prefer `by:`
> 
> When people talk about “sorting by XXX”, XXX is a property of the
> elements being sorted.  Therefore IMIO that label should probably be
> reserved for a version that takes a unary projection function:
> 
>     friends.sort(by: {$0.lastName})

But they're sorting now, and that feature isn't in Swift.

However, `sorted` falls under another umbrella, which is potential
chaining. In such case, I would prefer there be no internal label at all
to allow cleaner functional flow.

> 
>> -  if !expected.elementsEqual(actual, isEquivalent: sameValue) {
>> +  if !expected.elementsEqual(actual, comparingBy: sameValue) {
>> 
>> I'm torn on this one. I don't like but I don't have a good solution.

I actually meant to move this up to the other discussion of `byComparing` and
forgot to. So please assume you disagree with me on this one too.

>> 
>> -  /// for which `predicate(x) == true`.
>> +  /// for which `isIncluded(x) == true`.
>> -      _base: base.makeIterator(), whereElementsSatisfy: _include)
>> +      _base: base.makeIterator(), suchThat: _include)
>> 
>> How about simply `include` for both? 
> 
> Looking at the effect on the doc comments—which is how we should judge
> parameter names—I think `predicate` might read better.

I like `predicate`. I endorse `predicate`. Does this mean the rule of "must
read like a sentence" can be overturned for things like "comparison" and
"order"? If so, woo!

> 
>> I get the `is` desire but it's being tossed away in a lot of other
>> places in this diff. and `suchThat` feels out of place.
> 
> I think I'm pretty strongly sold on “soEach” as a label for closures in
> all the filtering components.

To quote greatness:, "I personally find `soEach` repulsive".
(cite: http://article.gmane.org/gmane.comp.lang.swift.evolution/16998/match=repulsive <http://article.gmane.org/gmane.comp.lang.swift.evolution/16998/match=repulsive>)

> 
>> -      || u16.contains({ $0 > 127 || _isspace_clocale($0) }) {
>> +    || u16.contains(elementWhere: { $0 > 127 || _isspace_clocale($0) }) {
>> 
>> I assume the challenge here is differentiating contains(element) from contains(closure).
>> This feels predicate-y, which is why I put it near the predicates. But I think something
>> like `containsElement(where:)` works better.
> 
> I understand your motivation for suggesting it.  The downside is that it
> severs the strong basename relationship between algorithms that do
> effectively the same things, one using a default comparison and the
> other using an explicitly-specified one.  I'm truly not sure where we
> ought to land on this one.

May I recommend `predicate:` then since it looks like that's actually a possibility?

> 
>> -    let result = try base._withUnsafeMutableBufferPointerIfSupported(body)
>> +    let result = try base._withUnsafeMutableBufferPointerIfSupported(invoke: body)
>> 
>> I hate "ifSupported" 
> 
> Me too.
> 
>> but that's another discussion
> 
> Quite.  It's also an internal API so it's not an evolution issue.  The
> point of having that change as part of the diff (as with all the other
> use sites), is to observe how the changes affect real usage.

Woody Allen: "The heart wants what it wants"
Me: "The spleen vents what it vents"

> 
>> (withSupportedUnsafeMutableBufferPointer,
>> withAvailableUnsafeMutableBufferPointer, it's all lipstick)
>> 
>> This is procedural, so `do` or `perform` rather than `invoke`
>> 
>> -      for test in removeFirstTests.filter({ $0.numberToRemove == 1 }) {
>> +      for test in removeFirstTests.filter(
>> +        suchThat: { $0.numberToRemove == 1 }
>> 
>> The difference between `filter` and `forEach` is that `forEach` is explicitly 
>> procedural while `filter` is functional.  I do not like functional chainable
>> calls being modified to use explicit external labels in this way. 
>> 
>> I'd prefer no label here.
> 
> Can you provide rationale for treating functional methods differently,
> or is it just personal taste?

Functional programming flow. I follow Kevin Ballard's rule of parens around
functional elements and naked braces for trailing closures that do not return
values. This ensures the compiler is never confused at places like:

for x in foo when y.f{...} {
    ...
}

and instantly identifies to the reader when there's a non-returning scope, as
in forEach or GCD dispatch.

However, if you use chaining and if the language insists on external label 
preambles, which would not be seen when using the call with a trailing closure,
it becomes far less readable and encourages people  to use trailing closures to 
avoid the label, i.e. an attractive nuisance. Simple selectors encourage better fp:

let x = myStream.f1({...}).f2({...})

> 
>> public func split(
>>      maxSplits: Int = Int.max,
>>      omittingEmptySubsequences: Bool = true,
>> -    isSeparator: @noescape (Base.Iterator.Element) throws -> Bool
>> +    separatedWhere isSeparator: @noescape (Base.Iterator.Element) throws -> Bool
>> 
>> I'm torn on this one. It's not the worst ever but something more like where/at/when
>> makes more sense to me than
>> "separatedWhere/separatedAt/separatedWhen".
> 
> The biggest reason to keep “separate” in the name is the connection with
> the semantics of the other `split` method, that takes a single element
> that is tested for equality.  I think this is very similar to the
> `contains(elementWhere` vs `containsElement(where` discussion.  If you
> leave “separate” out of the name it fails to imply that those elements
> for which the predicate returns true are not present in the result.

`predicate` ftw.

> 
>> 
>> +      count: __manager._headerPointer.pointee.count)
>> 
>> For the sake of Zippy the Pinhead, surely there has to be something better than `pointee`.
>> Like...`reference`?
> 
> It's not a reference; it's a value.  But that, my friend, is an
> *entirely* different discussion. Let's try to stick to the scope of the
> proposal: names and labels for parameters of function type, OK?

It was humor. It was at the end. I assumed the joke would lighten the
previous complaints and bookend the positive support at the start of
my message.

> 
> -- 
> -Dave

-- E


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160625/9b41c128/attachment.html>