<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 22, 2016 at 8:12 PM, Sean Heber <span dir="ltr"><<a href="mailto:sean@fifthace.com" target="_blank">sean@fifthace.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><div>How about:</div><div><br></div><div>let results = possibilities.where(matching: closure)</div></div></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><div><br></div><div>:)</div><div><br></div><div>l8r</div><div>Sean </div><div><br>Sent from my iPad</div><div><div class="h5"><div><br>On Jun 22, 2016, at 8:00 PM, Xiaodi Wu via swift-evolution <<a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a>> wrote:<br><br></div><blockquote type="cite"><div>filter(extractingWhere:)<br><div class="gmail_quote"><div dir="ltr">On Wed, Jun 22, 2016 at 18:53 Dave Abrahams via swift-evolution <<a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
on Wed Jun 22 2016, Xiaodi Wu <<a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a>> wrote:<br>
<br>
> I'll duly oblige with some pushback on `suchThat`. I get that you're trying<br>
> to clarify whether filter retains or gets rid of elements that match the<br>
> predicate, but I don't think "filter such that" expresses this idea at all.<br>
><br>
> Comparing to "filter where," "filter such that" is equally susceptible to<br>
> misinterpretation that you are filtering to remove elements that are<br>
> matched. For example: "find me some apples, filtering such that are<br>
> bruised."<br>
<br>
Hahaha, that's a very different interpretation of “such” that I hadn't<br>
considered! OK, suppose it was “soEach:” ?<br>
<br>
let primes = xs.filter(soEach: isPrime)<br>
<br>
> I'd suggest that if you want to be perfectly clear, you'd need something<br>
> like `filter(keepingWhere:)`.<br>
<br>
let primes = xs.filter(keepingWhere: isPrime)<br>
<br>
A slight problem is that filter is nonmutating, so all elements are<br>
“kept.” But maybe that's just Dave being overly concerned with unlikely<br>
misinterpretations at the cost of “naturalness.”<br>
<br>
Further thoughts?<br>
<br>
> On Wed, Jun 22, 2016 at 18:33 Dave Abrahams via swift-evolution <<br>
> <a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a>> wrote:<br>
><br>
>><br>
>> on Tue Jun 21 2016, Dave Abrahams <<a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a>> wrote:<br>
>><br>
>> > on Mon Jun 20 2016, Brent Royal-Gordon <<a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a>><br>
>> wrote:<br>
>> ><br>
>> >>> A couple of weeks ago we started to notice that we had some<br>
>> poorly-named<br>
>> >>> closure parameters and argument labels in the standard library, so we<br>
>> >>> did a complete audit of the standard library's APIs and came up with a<br>
>> >>> preliminary proposal for changes, which we applied in a branch and you<br>
>> >>> can review in <a href="https://github.com/apple/swift/pull/2981" rel="noreferrer" target="_blank">https://github.com/apple/swift/pull/2981</a>. Let's please<br>
>> >>> carry on further discussion here rather than in the pull request,<br>
>> though.<br>
>> >><br>
>> >> In general, I like this; `orderingBy` is a particularly nice<br>
>> >> improvement over the old `isOrderedBefore` convention.<br>
>> ><br>
>> > I don't really love the use of “by”, FWIW, but I thought `orderingWith`<br>
>> > was more confusable (ordering A with B might swap A and B, whereas the<br>
>> > parameter is a closure). It could be argued, though, that I am being<br>
>> > overly concerned with unlikely misinterpretations, at the cost of<br>
>> > “naturalness”—a known weakness of mine ;-). Anyway, as ever I'm open to<br>
>> > discussion on this.<br>
>> ><br>
>> >> A few specific comments about things I don't like:<br>
>> >><br>
>> >> * In `map` and `flatMap`, I'm not sure how much `transform` buys us<br>
>> >> over `elementTransform`.<br>
>> ><br>
>> > I think you mean the converse. And I agree that `elementTransform`<br>
>> > is probably not an improvement over `transform`.<br>
>><br>
>> ...and I've gone back to `transform` in my PR.<br>
>><br>
>> >> * In general, I'm not a fan of most of the changes away from `where`<br>
>> >> labels.<br>
>> ><br>
>> > The only such changes I can find are in<br>
>> ><br>
>> <a href="https://github.com/apple/swift/pull/2981/commits/3418eede88d724ad23731fe8f412f51e03cf5106" rel="noreferrer" target="_blank">https://github.com/apple/swift/pull/2981/commits/3418eede88d724ad23731fe8f412f51e03cf5106</a><br>
>> ><br>
>> > Note that part of this change was to make all filter closures<br>
>> > consistent; in the main `filter` API there was no label at all.<br>
>> > However, we felt that there's a real clarity problem with the polarity<br>
>> > of the argument (we talk about “filtering things out” but the closure<br>
>> > indicates which elements to keep). And we couldn't find a “where”-based<br>
>> > name that began to clarify it.<br>
>> ><br>
>> > I will argue that even changing to “suchThat,” as in the PR, does not<br>
>> > sufficiently clarify the closure's polarity, and the only true fix for<br>
>> > filter is to use a different base name (some have suggested “select,”<br>
>> > and I have other ideas), but that is out of scope for this particular<br>
>> > set of changes. So if the community is happier with a “where” label<br>
>> > here I can live with it. I do think “suchThat” is marginally clearer.<br>
>><br>
>> I have not received any further pushback on “suchThat,” so I've left it<br>
>> alone.<br>
>><br>
>> ><br>
>> >> Those are a nice, straightforward convention applied broadly across<br>
>> >> the Sequence APIs. (Yes, I criticized `where` as a method name in<br>
>> >> another thread, but I don't think `where` is a problem when there's a<br>
>> >> function base name to give it context.) When they don't work, that's<br>
>> >> usually because of a less-than-ideal base name. I'm not saying that<br>
>> >> *all* base names that aren't compatible with `where` should be<br>
>> >> changed, but rather that if `where` is not enough, that's an API<br>
>> >> smell.<br>
>> >><br>
>> >> * In particular, `elementWhere` is not a good label for the same<br>
>> >> reason that `removeElement` is not a good name. Session 403 last week<br>
>> >> actually talked about this between roughly minutes 8 and 11. (I'm sure<br>
>> >> you know about its content; you probably saw it before we did.)<br>
>> ><br>
>> > Yes I do, and I think you misinterpreted the message in that session.<br>
>> > There's nothing wrong with repeating type information when it's<br>
>> > necessary for clarity or fluency at the use-site. In the case of<br>
>> > `contains(elementWhere:)`, it's there for fluency:<br>
>> ><br>
>> > customers.contains(where: isSingle)<br>
>> ><br>
>> > doesn't read as well as:<br>
>> ><br>
>> > customers.contains(elementWhere: isSingle)<br>
>> ><br>
>> > The point is not to imagine that every argument should be preceded by<br>
>> > a noun, and repetition of type information is often the result of<br>
>> > trying to do that.<br>
>> ><br>
>> >> * I like `separatedWhere` on `split`, but I think the Equatable<br>
>> >> version needs a similar renaming.<br>
>> ><br>
>> > That's a nice thought; I think it's arguably out-of-scope here, though.<br>
>> ><br>
>> >> Perhaps `separatedBy`? `separatedOn`? The usual opposite of `where`,<br>
>> >> `of`, doesn't work here. (Alternatively, `separatedWhere` could be<br>
>> >> `separatorWhere` instead, but that's not quite as elegant.)<br>
>> ><br>
>> > I'd want to consider variations of `separatingAt` or `onSeparator` or<br>
>> > `atSeparator` too... which makes me thing “separatedWhere” might not be<br>
>> > as good as “separatingWhere” for the closure version.<br>
>> ><br>
>> >> * I'm very uncomfortable with the amount of weight<br>
>> >> `accumulatingResultBy` adds to `reduce`. `combinedBy` seems perfectly<br>
>> >> cromulent to me. I'm even more concerned by your suggestion in the<br>
>> >> pull request body of<br>
>> >> `accumulating(startingFrom:combiningBy:)`. `reduce` is a subtle and<br>
>> >> slightly confusing operation; adding more words to its call sites will<br>
>> >> not solve that problem. If you want to invent a new name from whole<br>
>> >> cloth, I would probably use something like `combining(with<br>
>> >> initialResult: T, by nextResult: (T, Element) -> T)`. (For that<br>
>> >> matter, while we're working in this area, `sequence(first:next:)`<br>
>> >> could use a similar coat of paint.)<br>
>> ><br>
>> > As with `filter(suchThat:`, `reduce(accumulatingResultBy:` is attempting<br>
>> > to solve with an argument label what IMO is a grave weakness in clarity<br>
>> > of the base name. If you read the documentation for `reduce`, you'll<br>
>> > see that it's all about accumulating a result, and if you consider that<br>
>> > its current signature often leads to O(N^2) behavior and we are thinking<br>
>> > about adding an overload that takes its “accumulator” inout, the<br>
>> > arguments for avoiding the name “accumulate” get progressively weaker.<br>
>> > But as noted earlier, changing base names is out-of-scope for this<br>
>> > proposal. As with “filter,” I could live with leaving this alone,<br>
>> > though I do believe “accumulatingResultBy:” is a real improvement in<br>
>> > clarity.<br>
>><br>
>> ...but I think it's overly specific at the expense of smoothness. So<br>
>> I've removed `Result` from that name.<br>
>><br>
>> >> * I agree with the comment on GitHub that `invoke` should be<br>
>> >> `execute`.<br>
>> ><br>
>> > Why? Rationales help.<br>
>> ><br>
>> >> If you see a distinction between the two cases on the number of<br>
>> >> arguments, I would then suggest `passTo` as the label on these<br>
>> >> methods: `views.forEach(passTo: addSubview)`,<br>
>> >> `withUnsafeBufferPointer(&bytes, passTo: Data.init(buffer:))`.<br>
>> ><br>
>> > Those are intriguing ideas, but that direction tends to suggest this<br>
>> > would be better:<br>
>> ><br>
>> > views.passEach(to: addSubview)<br>
>> > passUnsafeBufferPointer(to: Data.init(buffer:))<br>
>> ><br>
>> > ...until you pass a trailing closure:<br>
>> ><br>
>> > views.passEach { addSubView($0) }<br>
>> > passUnsafeBufferPointer { Data.init(buffer:$0) }<br>
>> ><br>
>> > (note: withUnsafeBufferPointer takes only one argument, a closure).<br>
>> ><br>
>> >><br>
>> >> * It's a little odd that you're using `comparingBy` for `Equatable`<br>
>> >> and `orderingBy` for `Comparable`. Did you judge `equatingBy` to be<br>
>> >> too awkward?<br>
>> ><br>
>> > Yes, and because it's not “equating,” which would mean using equality<br>
>> > (==) it's “testing equivalence” with respect to the predicate.<br>
>> ><br>
>> >> Perhaps the real problem is that `Equatable` ought to be `Comparable`<br>
>> >> and `Comparable` ought to be `Orderable`?<br>
>> ><br>
>> > I don't think so, personally, but regardless I consider such a change<br>
>> > out-of-scope for this proposal.<br>
>> ><br>
>> >> Or maybe `comparingBy` should just be something more general, like<br>
>> >> `matchingBy`? That would make perfectly sensible but slightly odd use<br>
>> >> cases like this one read better:<br>
>> >><br>
>> >> let isAnIdiot = luggageCombination.starts(with: [1, 2, 3, 4,<br>
>> >> 5], matchingBy: <=) // Matches [1,2,3,4,5], but also [1,1,1,1,1],<br>
>> >> [1,2,3,2,1], etc.<br>
>> ><br>
>> > That would not be legal, as <= is not an equivalence relation. You<br>
>> > could think about redefining the meaning of `starts(with:` to not<br>
>> > require an equivalence relation, but that's something I'm not confident<br>
>> > *I* know how to do meaningfully, and regardless is again out-of-scope.<br>
>> ><br>
>> >> Very soon (hopefully), I will be posting an early draft of a proposal<br>
>> >> renaming the various first/last/prefix/suffix/etc. APIs. I believe the<br>
>> >> only place it touches on your proposal is in<br>
>> >> `starts(with:isEquivalent:)`, but I think your changes to the second<br>
>> >> parameter label can be easily incorporated into what I'm doing.<br>
>> ><br>
>> > Great!<br>
>><br>
>> I'm going to write up the proposal ASAP based on the current PR unless I<br>
>> get more feedback.<br>
>><br>
>> Thanks,<br>
>><br>
>> --<br>
>> Dave<br>
>><br>
>> _______________________________________________<br>
>> swift-evolution mailing list<br>
>> <a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a><br>
>> <a href="https://lists.swift.org/mailman/listinfo/swift-evolution" rel="noreferrer" target="_blank">https://lists.swift.org/mailman/listinfo/swift-evolution</a><br>
>><br>
> _______________________________________________<br>
> swift-evolution mailing list<br>
> <a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a><br>
> <a href="https://lists.swift.org/mailman/listinfo/swift-evolution" rel="noreferrer" target="_blank">https://lists.swift.org/mailman/listinfo/swift-evolution</a><br>
><br>
<br>
--<br>
Dave<br>
<br>
_______________________________________________<br>
swift-evolution mailing list<br>
<a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a><br>
<a href="https://lists.swift.org/mailman/listinfo/swift-evolution" rel="noreferrer" target="_blank">https://lists.swift.org/mailman/listinfo/swift-evolution</a><br>
</blockquote></div>
</div></blockquote><blockquote type="cite"><div><span>_______________________________________________</span><br><span>swift-evolution mailing list</span><br><span><a href="mailto:swift-evolution@swift.org" target="_blank">swift-evolution@swift.org</a></span><br><span><a href="https://lists.swift.org/mailman/listinfo/swift-evolution" target="_blank">https://lists.swift.org/mailman/listinfo/swift-evolution</a></span><br></div></blockquote></div></div></div></blockquote></div><br></div></div>