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

[Xiaodi Wu]

On Sun, Jun 26, 2016 at 2:18 PM, Erica Sadun via swift-evolution <
swift-evolution at swift.org> wrote:

>
> On Jun 26, 2016, at 10:35 AM, Dave Abrahams <dabrahams at apple.com> wrote:
>
>
> on Sat Jun 25 2016, Erica Sadun <erica-AT-ericasadun.com
> <http://erica-at-ericasadun.com/>> wrote:
>
> 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
> <http://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) {
>
>
> OK, I understand your process now.  I still don't understand your
> rationale for saying `do` or `perform` is better than `invoke`.  Or is
> it just personal taste?
>
>
> See below.  But in a  nutshell, `do` gets the idea across. It's short.
> It's pithy.
> It uses a common, comfortable word. So yes, personal taste. But it's
> personal
> taste backed up by some statistics.
>

I've made this comment previously in another thread, but I'll chime in here
because it seems pertinent. Just like the parentheses around function
arguments can imply the word "with" or "using," and just as Swift 3 renamed
`foo(atIndex: Int)` to `foo(at: Int)` because an argument of type Int
implies an index, a closure to me implies the word "do," "invoke,"
"perform" or even "soEach." After all, it's absurd that a closure would be
supplied as an argument for any purpose other than being done or invoked or
performed. That a label such as `do` or `body` or `soEach` could be applied
so broadly seems to me a good sign that these labels might be extraneous.

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`.
>
>
> Ah, thanks.
>
> -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.
>
>
> I'm unable to reproduce those numbers using http://corpus.byu.edu/coca/.
> What exactly did you do to get them?
>
>
> I used a secondary webpage that rates word frequency based on the COCA
> corpus:
> http://www.wordfrequency.info/free.asp?s=y
>
>
> Also, I'm not sure commonality of use is a good rationale.  I bet
> “function” doesn't appear as often as “task” either, but the argument
> here is a function and we invoke functions.  Sometimes the appropriate
> word is just less common.
>
>
> 1051 task
> 1449 function
>
> Numbers aside, there's always a "term of art" argument to be made. A term
> of
> art is precise and instantly communicates the meaning to the user.
>
> I don't think a "term of art" argument can be made here since invoke,
> perform, and
> do all communicate the same idea to the end-coder, who doesn't need to
> know
> exactly how the compiler sees the argument.
>
> In naming theory, I'd say the member name carries the greatest weight when
> using
> exact terms and that internal labels should accessorize the primary name.
> When using a
> "term of art" argument, it should apply most strongly to a method/function/
> property/type name and less to supporting external labels.
>
> -  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.
>
>
> I plead guilty to sometimes having unnecessarily fussy tendencies, but
> in this case I believe strongly that “byComparing” would be actively
> misleading and harmful.  Even if there's only one possible sane
> interpretation, if readers have to scratch their heads and do an
> exhaustive search through possible interpretations of what something
> might mean because the one directly implied by the grammar is nonsense,
> that's bad.
>
>
> "Comparing by" is not actively misleading or harmful but it does sound
> strained.  I think a well-designed language should use comfortable
> constructs.Why not use a single , plain word that describes a parameter's
> role. Why not "compare:"? It's far less fussy and it's clear.
>
> 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:`.
>
>
> There is definitely something awkward about “xxxBy:” for these uses, and
> I'm open to using “xxxWith:” instead, even though as you say it's
> confusable.
>
>  if x.starts(with: y, comparingBy: { $0.name == $1.name }) {
>    ...
>  }
>
>  if x.starts(with: y, comparingWith: { $0.name == $1.name }) {
>    ...
>  }
>
> At some point I start to wonder if giving up fluency is best:
>
>  if x.starts(with: y, equality: { $0.name == $1.name }) {
>    ...
>  }
>
>
> Yes.  `predicate`, `compare` (or `comparison`), `order:`, etc.
>
> To riff off Gerard Manley Hopkins, "Glory be for simple things, for
> short words and trim APIs".
>
> 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.)
>
>
> I agree, but if you want concision I'd rather stick with something that
> doesn't imply anything unintended, such as “to” or “into.”
>
>
> I much prefer `to` and `into`
>
> -  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.
>
>
> That doesn't mean we should claim the syntax for another purpose.
> I think we probably do want that feature eventually.
>
> 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.
>
>
> IMO whether a method has a non-Void return value (and thus is chainable)
> should not be the determining feature on whether we use argument labels.
>
>
> If any method returns a sequence or collection, I think it *should* be
> considered
> in that specific light as a candidate for chaining. I agree that expanding
> that to
> any return value is too wide.
>
> We *could* decide to drop all labels for trailing closures (on the
> grounds that they often won't appear at the call site) and say the
> policy is that if there's something you want to communicate about the
> closure's role, you simply do the best you can with the parameter name
> and let the clarity at the call site fend for itself.  That, at least,
> would be a reasonably principled approach.
>
>
> I like labels for trailing closures when they are  `-> Void`.  You know a
> priori
> that the closure is used for side-effects, so if they are put into a
> function or
> method call, the label alerts you to design intent.
>
> I want to omit labels for trailing closures when they return collections,
> sequences,
> iterators, etc. because these return types are most likely to be chained.
>
> If you force a label for, say, map, people will choose
>
> map {  ...  }
>
> over
>
> map(label: { ... })
>
> because the former reads better, especially when chaining but it introduces
> bad habits, because it will not compile when used with constructs like:
>
> for i in map { ... } { // will not compile
>     ...
> }
>
> But if you treat collection/sequence fp as "highly likely to be chained",
> it encourages
> results more like:
>
> for i in x.map({ ...}).filter({ ... }) {
>     ....
> }
>
> which is compiler-safe, more readable, etc, especially for very short-form
> items like:
>
> for var i in x.flatMap({Int($0)}).filter({ $0 < 10 }) {
>     ...
> }
>
>
>
> -  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 don't know what you mean.  There is no such rule.  There are
> guidelines about reading like a phrase at the use site, but the
> *parameter name* has no effect on the use site.
>
> 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
> >)
>
>
> Sorry to hear that.  Let's change “filter” to “where” so this can become
> a non-issue.
>
>
> :)
>
>
>
> -      || 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?
>
>
> I never suggested predicate as an argument label.
>
> -    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"
>
>
> SOL (snort out loud)
>
>
> (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{...} {
>    ...
> }
>
>
> ? It's never confused there; it tells you it's illegal.
>
> and instantly identifies to the reader when there's a non-returning scope,
> as
> in forEach or GCD dispatch.
>
>
> Why is it useful to identify when there's a scope that doesn't return a
> value?  Isn't that already clear from whether the value is assigned
> somewhere?
>
>
> Enhanced readability from explicit coding intent. Trailing closures
> *look* like a scope created by a native construct. Those enclosed in
> parens *look* like arguments.
>
> -- E
>
>
> _______________________________________________
> 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/20160627/9d7dd795/attachment.html>