> 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. 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`.
* In general, I'm not a fan of most of the changes away from `where` labels.
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.)
* I like `separatedWhere` on `split`, but I think the Equatable version needs a
similar renaming. 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'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.)
* I agree with the comment on GitHub that `invoke` should be `execute`. 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:))`.
* It's a little odd that you're using `comparingBy` for `Equatable` and
`orderingBy` for `Comparable`. Did you judge `equatingBy` to be too awkward?
Perhaps the real problem is that `Equatable` ought to be `Comparable` and
`Comparable` ought to be `Orderable`? 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.
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.
--
Brent Royal-Gordon
Architechies
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution
