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

[Erica Sadun]

On Jun 20, 2016, at 3:25 PM, Dave Abrahams via swift-evolution <swift-evolution at swift.org> wrote:
> 
> 
> Hi All,
> 
> 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.

-  /// - `isEquivalent(a, a)` is always `true`. (Reflexivity)
 -  /// - `isEquivalent(a, b)` implies `isEquivalent(b, a)`. (Symmetry)
 -  /// - If `isEquivalent(a, b)` and `isEquivalent(b, c)` are both `true`, then
 -  ///   `isEquivalent(a, c)` is also `true`. (Transitivity)
 +  /// - `areEquivalent(a, a)` is always `true`. (Reflexivity)
 +  /// - `areEquivalent(a, b)` implies `areEquivalent(b, a)`. (Symmetry)
 +  /// - If `areEquivalent(a, b)` and `areEquivalent(b, c)` are both `true`, then
 +  ///   `areEquivalent(a, c)` is also `true`. (Transitivity)

I like this change!

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

That said, would prefer `do` or `perform` over `invoke` or `invoking` as in
`runRaceTest`, `_forAllPermutationsImpl`, `expectFailure`, etc. This also applies
where there's a `body` label instead of an empty external label.

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

-  return IteratorSequence(it).reduce(initial, combine: f)
+  return IteratorSequence(it).reduce(initial, accumulatingBy: f)

For `reduce`, I'd prefer `applying:` or `byApplying:`

Similarly in `starts(with:comparingBy:)`, I'd prefer byComparing`,
min/max, byOrdering

-      ).encode(encoding, output: output)
+      ).encode(encoding, sendingOutputTo: processCodeUnit)

How about `exportingTo`?


-  tempwords.sort(isOrderedBefore: <)
+  tempwords.sort(orderingBy: <)

With `sort` and `sorted`, I'd prefer `by:`

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

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

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

 -    let result = try base._withUnsafeMutableBufferPointerIfSupported(body)
+    let result = try base._withUnsafeMutableBufferPointerIfSupported(invoke: body)

I hate "ifSupported" but that's another discussion (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.

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

+      count: __manager._headerPointer.pointee.count)

For the sake of Zippy the Pinhead, surely there has to be something better than `pointee`.
Like...`reference`?


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