[swift-evolution] [Review] SE-0065 A New Model for Collections and Indices

Mon Apr 11 21:36:55 CDT 2016

Proposal link: https://github.com/apple/swift-evolution/blob/master/proposals/0065-collections-move-indices.md

> On Apr 11, 2016, at 2:59 PM, Dave Abrahams via swift-evolution <swift-evolution at swift.org> wrote:
> 
> Thanks for your comments, Brent!
> 
> on Sun Apr 10 2016, Brent Royal-Gordon <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:

The shift described in this proposal is extremely valuable and makes implementing collections far more intuitive, as all the collection's logic lives "inside" the collection itself. My only hesitation is with the naming of the method that Brent also called out:

... snip ...

>>> func index(n: IndexDistance, stepsFrom i: Index) -> Index
>> 
>> Oof, I am really not a fan of this name. `steps` is sort-of a label on
>> the `n` parameter, but it's attached to `i`. 

Oof indeed! This is a very unusual method in the standard library, since we're calling on one instance to perform an action on another. My problems with the naming are twofold: 

(1) Collision with the index(of:) and index(where:) APIs
The existing methods are used for searching a collection, possibly finding a matching index, possibly not. The new ones deterministically find an new index at a prescribed distance, with important and slightly complicated preconditions. These differences make the use and "flavor" of the two sets of methods distinct enough that I think they should have different names.

(2) Arguments are reversed
I think the ideal API for this would be index.advanced(by: 5, in: c), but I prefer keeping the index-moving implementation in the collection, not the index. I would favor any naming for this method that puts the index before the distance, keeping the overall shape of the advanced(by:) method. c.advance(i, by: 4) would be my pick.

....and finally I'll just go ahead and say again that I prefer -InPlace over form-. That's all, I'm done!

Nate

ps. Seriously, collections make so much more sense with this change. +1000

> Yes, it's an awkward thing to name.  Better suggestions most welcome.
> 
>> Other collection APIs use `distance`, not `steps` (although "steps"
>> does appear in the documentation of the `Distance` associated
>> type). `index` puts it in a method family with `index(predicate:)` and
>> `index(of:)`, but those two are user-facing while this one is part of
>> the collection API. Even the word `index` itself is redundant with the
>> method return type.
>> 
>> I do understand how this is kind of parallel to `index(of:)` and
>> `index(predicate:)`, in that they all return an index calculated from
>> the parameters, but I think these methods are more different than they
>> are similar.
>> 
>> Compared to this:
>> 
>> 	collection.index(5, stepsFrom: i)
>> 
>> I would prefer any of these (ordered from least favorite to most):
>> 
>> 	collection.index(distance: 5, from: i)
> 
> I'm OK with this one, but am not sure it's an improvement over the
> proposal.  I'd like to hear other peoples' arguments on this.
> 
>> 	collection.index(5, from: i)
> 
> I don't think this one reads clearly enough.
> 
>> 	collection.traveling(5, from: i)
>> 	collection.striding(5, from: i)
>> 	collection.advancing(i, by: 5)
> 
> None of the “ing” names work, IMO because that suffix suggests you're
> returning a modified version of the receiver.
> 
>> A word on `striding(_:from:)` appearing in that list: Although
>> redesigning Strideable is not directly in scope for this proposal,
>> I've noticed that our discussions on modernizing Strideable seem to be
>> trending towards the idea that it operates on collections (or rather,
>> on an as-yet-unnamed supertype of `BidirectionalCollection` or
>> `RandomAccessCollection`) and strides by repeatedly calling a method
>> with the same semantics as this one. Thus, there seems to be an
>> intimate connection between this operation and Strideable. I think we
>> ought to choose a method name which suits that, and I don't think
>> `index` is it.
>> 
>>> func index(n: IndexDistance, stepsFrom i: Index, limitedBy limit: Index) -> Index
>> 
>> I have a few issues with this API.
>> 
>> 1. As aforementioned, I'm not a big fan of using `index` as the base method name.
>> 
>> 2. This method can move the index either forwards or backwards, but
>> only one limit is specified. Would we be better off having the `limit`
>> be a range?
> 
> That would add a cost for checking that one doesn't want to pay in
> algorithms that need this method.
> 
>> 3. What is the use case for returning the `limit` instead of returning
>> the fact that we crossed it? I have a hard time thinking of a case
>> where I would want to just bump up against the limit and use it rather
>> than *detect* that I've hit the limit (which would probably call for a
>> return type of `Index?`). Are there common needs that I'm just not
>> thinking of? 
> 
> Sure, for example
> 
>  x[i..<x.index(n, stepsFrom: i, limitedBy: x.endIndex)].sort()
> 
> 
>> Should we offer both?
> 
> Definitely not, IMO!  They are utterly redundant, are they not?
> 
>> 
>>> 	* What is your evaluation of the proposal?
>> 
>> Despite my criticisms, this is fundamentally a very good design. It
>> will not only improve the language, it will also open the door to
>> further improvements.
>> 
>>> 	* Is the problem being addressed significant enough to warrant a change to Swift?
>> 
>> Yes. I believe this change is complicating in the short run but
>> actually simplifying in the long run, eliminating concepts like the
>> Index protocols which represented several overlapping semantics.
>> 
>>> 	* 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?
>> 
>> Nothing with a collection design as rich as Swift's.
>> 
>>> 	* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
>> 
>> Somewhere between the latter two. I wouldn't call it in-depth when
>> it's such a big change, but I feel like I have too much background to
>> say it's a quick reading, either.
> 
> -- 
> Dave
> 
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160411/7f9b9584/attachment-0001.html>