+1 to Tony's arguments and to increment/decrement -Thorsten
> Am 13.04.2016 um 17:45 schrieb Tony Parker via swift-evolution > <[email protected]>: > > >> On Apr 12, 2016, at 3:43 PM, Dave Abrahams via swift-evolution >> <[email protected]> wrote: >> >> >> Thanks for your review, Tony! >> >> on Mon Apr 11 2016, Tony Parker <[email protected]> wrote: >> >>>> On Apr 10, 2016, at 2:41 PM, Chris Lattner via swift-evolution >>>> <[email protected]> wrote: >>>> >>>> Hello Swift community, >>>> >>>> The review of "A New Model for Collections and Indices" begins now and >>>> runs through April 18th. The proposal is available here: >>> >>>> >>>> >>>> https://github.com/apple/swift-evolution/blob/master/proposals/0065-collections-move-indices.md >>>> >>>> Reviews are an important part of the Swift evolution process. All reviews >>>> should be sent to the swift-evolution mailing list at: >>>> https://lists.swift.org/mailman/listinfo/swift-evolution >>>> or, if you would like to keep your feedback private, directly to the >>>> review manager. >>>> >>>> >>>> What goes into a review? >>>> >>>> The goal of the review process is to improve the proposal under >>>> review through constructive criticism and, eventually, determine the >>>> direction of Swift. When writing your review, here are some >>>> questions you might want to answer in your review: >>>> >>>> * What is your evaluation of the proposal? >>> >>> I agree with the general direction and scope of the proposal, but I >>> think the names could use some changes. Specifically, I don’t think >>> the fallback to ‘form’ is required. >> >> It's not a fallback whatsoever. The updated guidelines referenced from >> https://github.com/apple/swift-evolution/blob/master/proposals/0059-updated-set-apis.md >> (which was accepted this morning; announcement forthcoming) make the >> “form” prefix a first-class citizen that one chooses based on the >> noun-ness of the underlying operation. Furthermore, *treating* >> “formXXX” as a fallback and avoiding it will continue to strengthen the >> sense that it's not something we should normally use, leading to more >> naming arguments in the future. It's a strong guideline and as long as >> we have it, we shouldn't be afraid to apply it, thereby increasing >> uniformity and predictability. >> >> [To all my fellow “InPlace” lovers out there: yes, another guideline >> might be more optimal, but this is the guideline we have/can get]. > > In other cases, the mutating pair of methods refer to the receiver, not the > argument. > > x = y.union(z) // new value x > y.formUnion(z) // mutates y, not z > > x = y.successor(z) // new value x > y.formSuccessor(z) // mutates z (or replaces), not y > > I think using the form prefix here will confuse this case with the others, > when they are meaningfully different. > >>> It would be a significant readability improvement to use a meaningful >>> verb to describe the action of altering the argument. The methods that >>> create new indices probably need a label on the first argument, >>> because otherwise it looks as if the IndexDistance is what is >>> described by ‘index’. >>> >>> Proposed: >>> >>> func successor(of i: Index) -> Index >>> func formSuccessor(i: inout Index) >>> >>> Instead, I suggest: >>> >>> func successor(of i : Index) -> Index >>> func advance(i: inout Index) >> >> Why is that an improvement? It loses the correspondence between the >> operations, which are still a mutating/nonmutating pair. What's it got >> to recommend it? I have the same question about all of the suggestions >> below. > > It’s an improvement because it is much easier to read and understand what it > means. The phrase “form successor” only makes sense if you dive into the > naming guidelines to see why we have the “form” prefix in the first place. > Plus, as I said, the form prefix implies a mutation of the wrong argument. > >> >>> Proposed: >>> >>> func index(n: IndexDistance, stepsFrom i: Index) -> Index >>> func index(n: IndexDistance, stepsFrom i: Index, limitedBy limit: Index) >>> -> Index >>> func formIndex(n: IndexDistance, stepsFrom i: inout Index) >>> func formIndex(n: IndexDistance, stepsFrom i: inout Index, limitedBy >>> limit: Index) >>> >>> Suggested (taking into account Nate’s suggestion of reversing the order): >>> >>> func index(startingAt i: Index, movedBy n: IndexDistance) -> Index >>> func index(startingAt i: Index, movedBy n: IndexDistance, limitedBy limit: >>> Index) -> Index >> >> I find Nate Cook's concerns about the use of “index” here (a mental >> clash with unrelated methods having the same basename) especially >> convincing. So I think I want to look for other names for these. >> >>> >>> func move(i : inout Index, by n: IndexDistance) >>> func move(i : inout Index, by n: IndexDistance, limitedBy limit: Index) >>> >>> Proposed: >>> >>> func predecessor(of i: Index) -> Index >>> func formPredecessor(i: inout Index) >>> >>> Suggested: >>> >>> func predecessor(of i: Index) -> Index >>> func reverse(i: inout Index) >>> >>> I think reversing an index has some nice symmetry with reversing a >>> sequence, but if it seems to confusing, then replace advance and >>> reverse with ‘moveForward’ and ‘moveBackward’. >> >> Yeah, I don't think moving an index one step backwards could reasonably >> be called “reversing” it. “moveBackward” is reasonable, if one wanted >> have to break the relationship with predecessor. > > Reverse is the best opposite we have of advance, so it makes sense to me. Or > we could use retreat. =) There are other pairs of words that work as well, > like “increment/decrement”. It’s rather an implementation detail of the index > and collection what exactly these do, but conceptually modeling them as > increment and decrement would likely make intuitive sense to most CS 101 > students. > > The reason that advance/reverse and increment/decrement work better is > because they are active words that describe what happens to the argument, > which has no other label. “form” describes little about the action that is > actually taken on the argument. Therefore, to me it feels like a placeholder > or fallback. > > - Tony > >>> >>> >>> - Tony >>> >>>> * Is the problem being addressed significant enough to warrant a change >>>> to Swift? >>>> * Does this proposal fit well with the feel and direction of Swift? >>>> * If you have you used other languages or libraries with a similar >>>> feature, how do you feel that this proposal compares to those? >>>> * How much effort did you put into your review? A glance, a quick >>>> reading, or an in-depth study? >>>> >>>> More information about the Swift evolution process is available at >>>> >>>> https://github.com/apple/swift-evolution/blob/master/process.md >>>> >>>> Thank you, >>>> >>>> -Chris Lattner >>>> Review Manager >>>> >>>> >>>> _______________________________________________ >>>> 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
_______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
