FWIW, I'm posting this review on behalf of Dmitri Gribenko, and Maxim Moiseev, and me:
on Thu Apr 28 2016, Chris Lattner <[email protected]> wrote: > Hello Swift community, > > The review of "SE-0045: Add scan, prefix(while:), drop(while:), and > iterate to the stdlib" begins now and runs through May 3. The proposal > is available here: > > > https://github.com/apple/swift-evolution/blob/master/proposals/0045-scan-takewhile-dropwhile.md > > * What is your evaluation of the proposal? We like the bones, but we have some issues with the proposal as submitted: 1. The proposal should be revised to account for language and standard library changes, most notably, the first argument label rules and API names have been overhauled since the proposal was written. There are still many occurrences of obsolete names such as `SequenceType`. One good way to get there is to make the APIs compile (with `fatalError()` in method bodies if necessary) against the master branch in GitHub. I realize it's been a long time since it was originally submitted, but it's really hard to evaluate a proposal if we'd have to change the APIs before they could even be incorporated into the codebase. The fact that names are out of date also casts doubt upon whether a first argument label is being proposed or not. IMO it's really crucial that the proposal be revised this way *before* it is accepted. 2. In the “Detailed design” section it would have helped to have an explanation of the remark about “matching expected behavior.” We were scratching our heads over this one until we figured out it’s about the policy of not making multiple passes using a user-defined closure without an explicit appearance of `.lazy`. 3. We would prefer that `scan`’s first argument label was changed from `initial` to `startingWith`, and we would like to update `reduce` to use the same argument label. Whatever we do, `reduce` and `scan` should be consistent in this respect. 4. We find the name `iterate` problematic, in part because it is an active verb on a non-mutating method, but also because to us, it strongly implies eager evaluation. In lieu of `iterate`, we'd prefer to see two overloads of `unfold` as shown below. Although `unfold` is also an active verb, it is very much the correct term of art for the more general method and it allows us to establish the semantic relationship between the single- and two-argument forms. func unfold<T, State>( _ initialState: State, applying nextElementAndState: (State)->(T, State)? ) -> UnfoldSequence<T> func unfold<T>( _ initialElement: T, applying nextElement: (T)->T? ) -> UnfoldSequence<T> The second overload returns, approximately, unfold(initialElement) { state in nextElement(state).map { (state, $0) } } though that implementation would be a bit more eager than necessary; a fully-lazy implementation is at https://github.com/apple/swift-evolution/pull/195#issuecomment-214927063 > * Is the problem being addressed significant enough to warrant a > change to Swift? Yes, it is crucial that Swift have a complete vocabulary of high-level algorithms. While these are simple, they are also fundamental missing pieces. > * Does this proposal fit well with the feel and direction of > Swift? Yes. > * If you have you used other languages or libraries with > a similar feature, how do you feel that this proposal compares > to those? We don't have anything to say as a group about this, but Max or Dmitri may have individual feedback. > * How much effort did you put into your review? A > glance, a quick reading, or an in-depth study? An in-depth, collaborative, 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 -- Dave, Dmitri, and Maxim _______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
