[swift-evolution] [swift-evolution-announce] [Review] SE-0118: Closure Parameter Names and Labels

[Dave Abrahams]

on Thu Jul 07 2016, Jordan Rose <jordan_rose-AT-apple.com> wrote:

> [Proposal: https://github.com/apple/swift-evolution/blob/master/proposals/0118-closure-parameter-names-and-labels.md ]
>
> Hi, Dave, Dmitri, Max. Sorry I didn’t make the pre-review commentary
> thread. I find I’m still not happy with several of these names,
> although there are many other improvements. Stepping back, I think the
> Swift API guidelines just don’t do a good job with closure
> parameters. We should have naming guidelines for strategy closures and
> for callbacks.
>
> lines.split(whereSeparator: isAllWhitespace)
>
> This reads like an English sentence, but it doesn’t have the correct
> meaning for me. This implies a structure that has a pre-existing
> “separator", and checks if that separator matches the predicate,
> rather than searching for an element that matches the predicate, and
> splitting on that. I realize that the former reading doesn’t make much
> sense as a function, but it’s still impeded my understanding more than
> helping it along.
>
> Alternate suggestions: split(where:), split(separatingWhere:).

Split(where:) fails to imply that there are separators (and that some
elements would be omitted from the result), but we considered the second
one.  I like the way it reads better, but “whereSeparator” has the
advantag of containing the word “separator” that's used in the
predicate-free version.  For me it's a bit of a toss-up.

> The sort(by:) family
>
> …seems fine, is slightly confusable with a “sort by key” API, but at
> least has a different type signature.

Yes, well, fortunately, Bool is not Comparable, or we would have a
problem when we want to introduce that API.

> if roots.contains(where: isPrime) {
>
> I really don’t like this one; it feels like it’s missing a noun. It
> certainly isn’t a valid English phrase. If we were naming an
> intersects(_:)-like feature, we might call it `roots.containsAny(of:
> primes)` or `roots.contains(anyOf: primes)`.

I agree.  
>
> Alternate suggestions: containsAny(where:), contains(anyWhere:)

The second one, IMO.

> if expected.elementsEqual(actual, by: haveSameValue)
>
> I tend to agree with the pre-review commentary
> <http://thread.gmane.org/gmane.comp.lang.swift.evolution/22188> that
> “by:” doesn’t actually fit here. The difference between
> elementsEqual(_:by:) / starts(with:by:) and the sort(by:) family is
> that “sort by…” and “order by…” are both things people actually say,
> but “equal by…” is not. Then again, when they say those things,
> they’re either talking about a key (“sort by name”) or a general
> comparison (“sort by compar[ing] names”, <=> rather than <).

  names1.elementsEqual(names2, by: {$0.lowercase == $1.lowercase})


reads OK to me.

> let sum = measurements.reduce(0, +)
>
> It concerns me that we can’t think of a label for this operation; it
> implies we can’t think of an English sentence to describe it. “Reduce
> this collection to a single element by combining using +.”
> reduce(startingWith:combiningWith:) is pretty unwieldy, though.

IMO, if you read the doc comments for reduce, they push pretty strongly
toward renaming it to “accumulate,” if you can tolerate the loss of the
term-of-art.  That, and the traditional usage-of-art, make me
unconcerned that there's no good label while it's called “reduce.”
FWIW.

-- 
Dave