on Wed Jun 22 2016, Xiaodi Wu <xiaodi.wu-AT-gmail.com> wrote: > On Wed, Jun 22, 2016 at 8:12 PM, Sean Heber <[email protected]> wrote: > >> How about: >> >> let results = possibilities.where(matching: closure) >> > > My understanding is that we're restricting the bikeshedding here to the > closure label. Renaming `filter` is discussed in another thread, though > obviously that would change the name of the label as well.
Correct, thanks. >> >> :) >> >> l8r >> Sean >> >> Sent from my iPad >> >> On Jun 22, 2016, at 8:00 PM, Xiaodi Wu via swift-evolution < >> [email protected]> wrote: >> >> filter(extractingWhere:) >> On Wed, Jun 22, 2016 at 18:53 Dave Abrahams via swift-evolution < >> [email protected]> wrote: >> >>> >>> on Wed Jun 22 2016, Xiaodi Wu <[email protected]> wrote: >>> >>> > I'll duly oblige with some pushback on `suchThat`. I get that you're >>> trying >>> > to clarify whether filter retains or gets rid of elements that match the >>> > predicate, but I don't think "filter such that" expresses this idea at >>> all. >>> > >>> > Comparing to "filter where," "filter such that" is equally susceptible >>> to >>> > misinterpretation that you are filtering to remove elements that are >>> > matched. For example: "find me some apples, filtering such that are >>> > bruised." >>> >>> Hahaha, that's a very different interpretation of “such” that I hadn't >>> considered! OK, suppose it was “soEach:” ? >>> >>> let primes = xs.filter(soEach: isPrime) >>> >>> > I'd suggest that if you want to be perfectly clear, you'd need something >>> > like `filter(keepingWhere:)`. >>> >>> let primes = xs.filter(keepingWhere: isPrime) >>> >>> A slight problem is that filter is nonmutating, so all elements are >>> “kept.” But maybe that's just Dave being overly concerned with unlikely >>> misinterpretations at the cost of “naturalness.” >>> >>> Further thoughts? >>> >>> > On Wed, Jun 22, 2016 at 18:33 Dave Abrahams via swift-evolution < >>> > [email protected]> wrote: >>> > >>> >> >>> >> on Tue Jun 21 2016, Dave Abrahams <[email protected]> wrote: >>> >> >>> >> > on Mon Jun 20 2016, Brent Royal-Gordon <[email protected]> >>> >> wrote: >>> >> > >>> >> >>> 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. >>> >> > >>> >> > I don't really love the use of “by”, FWIW, but I thought >>> `orderingWith` >>> >> > was more confusable (ordering A with B might swap A and B, whereas >>> the >>> >> > parameter is a closure). It could be argued, though, that I am being >>> >> > overly concerned with unlikely misinterpretations, at the cost of >>> >> > “naturalness”—a known weakness of mine ;-). Anyway, as ever I'm >>> open to >>> >> > discussion on this. >>> >> > >>> >> >> 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`. >>> >> > >>> >> > I think you mean the converse. And I agree that `elementTransform` >>> >> > is probably not an improvement over `transform`. >>> >> >>> >> ...and I've gone back to `transform` in my PR. >>> >> >>> >> >> * In general, I'm not a fan of most of the changes away from `where` >>> >> >> labels. >>> >> > >>> >> > The only such changes I can find are in >>> >> > >>> >> >>> https://github.com/apple/swift/pull/2981/commits/3418eede88d724ad23731fe8f412f51e03cf5106 >>> >> > >>> >> > Note that part of this change was to make all filter closures >>> >> > consistent; in the main `filter` API there was no label at all. >>> >> > However, we felt that there's a real clarity problem with the >>> polarity >>> >> > of the argument (we talk about “filtering things out” but the closure >>> >> > indicates which elements to keep). And we couldn't find a >>> “where”-based >>> >> > name that began to clarify it. >>> >> > >>> >> > I will argue that even changing to “suchThat,” as in the PR, does not >>> >> > sufficiently clarify the closure's polarity, and the only true fix >>> for >>> >> > filter is to use a different base name (some have suggested “select,” >>> >> > and I have other ideas), but that is out of scope for this particular >>> >> > set of changes. So if the community is happier with a “where” label >>> >> > here I can live with it. I do think “suchThat” is marginally >>> clearer. >>> >> >>> >> I have not received any further pushback on “suchThat,” so I've left it >>> >> alone. >>> >> >>> >> > >>> >> >> 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.) >>> >> > >>> >> > Yes I do, and I think you misinterpreted the message in that session. >>> >> > There's nothing wrong with repeating type information when it's >>> >> > necessary for clarity or fluency at the use-site. In the case of >>> >> > `contains(elementWhere:)`, it's there for fluency: >>> >> > >>> >> > customers.contains(where: isSingle) >>> >> > >>> >> > doesn't read as well as: >>> >> > >>> >> > customers.contains(elementWhere: isSingle) >>> >> > >>> >> > The point is not to imagine that every argument should be preceded by >>> >> > a noun, and repetition of type information is often the result of >>> >> > trying to do that. >>> >> > >>> >> >> * I like `separatedWhere` on `split`, but I think the Equatable >>> >> >> version needs a similar renaming. >>> >> > >>> >> > That's a nice thought; I think it's arguably out-of-scope here, >>> though. >>> >> > >>> >> >> 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'd want to consider variations of `separatingAt` or `onSeparator` or >>> >> > `atSeparator` too... which makes me thing “separatedWhere” might not >>> be >>> >> > as good as “separatingWhere” for the closure version. >>> >> > >>> >> >> * 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.) >>> >> > >>> >> > As with `filter(suchThat:`, `reduce(accumulatingResultBy:` is >>> attempting >>> >> > to solve with an argument label what IMO is a grave weakness in >>> clarity >>> >> > of the base name. If you read the documentation for `reduce`, you'll >>> >> > see that it's all about accumulating a result, and if you consider >>> that >>> >> > its current signature often leads to O(N^2) behavior and we are >>> thinking >>> >> > about adding an overload that takes its “accumulator” inout, the >>> >> > arguments for avoiding the name “accumulate” get progressively >>> weaker. >>> >> > But as noted earlier, changing base names is out-of-scope for this >>> >> > proposal. As with “filter,” I could live with leaving this alone, >>> >> > though I do believe “accumulatingResultBy:” is a real improvement in >>> >> > clarity. >>> >> >>> >> ...but I think it's overly specific at the expense of smoothness. So >>> >> I've removed `Result` from that name. >>> >> >>> >> >> * I agree with the comment on GitHub that `invoke` should be >>> >> >> `execute`. >>> >> > >>> >> > Why? Rationales help. >>> >> > >>> >> >> 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:))`. >>> >> > >>> >> > Those are intriguing ideas, but that direction tends to suggest this >>> >> > would be better: >>> >> > >>> >> > views.passEach(to: addSubview) >>> >> > passUnsafeBufferPointer(to: Data.init(buffer:)) >>> >> > >>> >> > ...until you pass a trailing closure: >>> >> > >>> >> > views.passEach { addSubView($0) } >>> >> > passUnsafeBufferPointer { Data.init(buffer:$0) } >>> >> > >>> >> > (note: withUnsafeBufferPointer takes only one argument, a closure). >>> >> > >>> >> >> >>> >> >> * It's a little odd that you're using `comparingBy` for `Equatable` >>> >> >> and `orderingBy` for `Comparable`. Did you judge `equatingBy` to be >>> >> >> too awkward? >>> >> > >>> >> > Yes, and because it's not “equating,” which would mean using equality >>> >> > (==) it's “testing equivalence” with respect to the predicate. >>> >> > >>> >> >> Perhaps the real problem is that `Equatable` ought to be >>> `Comparable` >>> >> >> and `Comparable` ought to be `Orderable`? >>> >> > >>> >> > I don't think so, personally, but regardless I consider such a change >>> >> > out-of-scope for this proposal. >>> >> > >>> >> >> 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. >>> >> > >>> >> > That would not be legal, as <= is not an equivalence relation. You >>> >> > could think about redefining the meaning of `starts(with:` to not >>> >> > require an equivalence relation, but that's something I'm not >>> confident >>> >> > *I* know how to do meaningfully, and regardless is again >>> out-of-scope. >>> >> > >>> >> >> 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. >>> >> > >>> >> > Great! >>> >> >>> >> I'm going to write up the proposal ASAP based on the current PR unless >>> I >>> >> get more feedback. >>> >> >>> >> Thanks, >>> >> >>> >> -- >>> >> Dave >>> >> >>> >> _______________________________________________ >>> >> swift-evolution mailing list >>> >> [email protected] >>> >> https://lists.swift.org/mailman/listinfo/swift-evolution >>> >> >>> > _______________________________________________ >>> > swift-evolution mailing list >>> > [email protected] >>> > https://lists.swift.org/mailman/listinfo/swift-evolution >>> > >>> >>> -- >>> Dave >>> >>> _______________________________________________ >>> swift-evolution mailing list >>> [email protected] >>> https://lists.swift.org/mailman/listinfo/swift-evolution >>> >> _______________________________________________ >> swift-evolution mailing list >> [email protected] >> https://lists.swift.org/mailman/listinfo/swift-evolution >> >> -- -Dave _______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
