[Review] SE-0078: Implement a rotate algorithm, equivalent to std::rotate() in C++


(Xiaodi Wu) #1

        * What is your evaluation of the proposal?

+1. A very useful function to have on collections.

In FORTRAN and in languages that take inspiration from it, the same
function is called a "circular shift"--IMO, it's a clearer name both by its
associations with other uses of the term "shifting" and because googling
the term will give much more useful results.

IMO, the parameter label could also use some bikeshedding. One reason is
that, with an `Array<Int>` (as illustrated in the examples given), the
argument in `array.rotate(firstFrom: 5)` does not obviously refer to an
index. Superficially, this could mean first from the first element equal to
5.

One suggestion I would put forward is: `circularShift(leftBy:
IndexDistance)`. For bidirectional and random access collections, this
function could possibly be complemented by `circularShift(rightBy:
IndexDistance)`.

Finally, I wonder whether the C++11-like behavior of returning the new
index of the former first item is a very Swifty behavior. I could be
persuaded if the return value is demonstrably something that is not always
trivial to compute.

        * Is the problem being addressed significant enough to warrant a

change to Swift?

It's an additive feature, and it's hard to see any drawbacks.

        * Does this proposal fit well with the feel and direction of Swift?

Yes, it's an algorithm of appropriate complexity and general utility to be
added to the stdlib.

        * If you have used other languages or libraries with a similar
feature, how do you feel that this proposal compares to those?

This proposal presents the same algorithm as found in other languages but
borrows the name from C++. Other names may be more self-documenting (see
above).

        * How much effort did you put into your review? A glance, a quick
reading, or an in-depth study?

I have read the proposal to a medium depth.


(Nate Cook) #2

        * What is your evaluation of the proposal?

+1. A very useful function to have on collections.

In FORTRAN and in languages that take inspiration from it, the same function is called a "circular shift"--IMO, it's a clearer name both by its associations with other uses of the term "shifting" and because googling the term will give much more useful results.

IMO, the parameter label could also use some bikeshedding. One reason is that, with an `Array<Int>` (as illustrated in the examples given), the argument in `array.rotate(firstFrom: 5)` does not obviously refer to an index. Superficially, this could mean first from the first element equal to 5.

One suggestion I would put forward is: `circularShift(leftBy: IndexDistance)`. For bidirectional and random access collections, this function could possibly be complemented by `circularShift(rightBy: IndexDistance)`.

Finally, I wonder whether the C++11-like behavior of returning the new index of the former first item is a very Swifty behavior. I could be persuaded if the return value is demonstrably something that is not always trivial to compute.

For random-access collections, calculating the new index is trivial, but for forward or bidirectional collections it's an O(n) operation if not calculated as part of the rotation. For the same reason, it's important that the parameter is an index and not just a distance.

Nate

···

On May 3, 2016, at 11:51 PM, Xiaodi Wu via swift-evolution <swift-evolution@swift.org> wrote:

        * Is the problem being addressed significant enough to warrant a change to Swift?

It's an additive feature, and it's hard to see any drawbacks.

        * Does this proposal fit well with the feel and direction of Swift?

Yes, it's an algorithm of appropriate complexity and general utility to be added to the stdlib.

        * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

This proposal presents the same algorithm as found in other languages but borrows the name from C++. Other names may be more self-documenting (see above).

        * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I have read the proposal to a medium depth.
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution