-
Notifications
You must be signed in to change notification settings - Fork 32
New circular Seq operations: rotateRight, rotateLeft, startAt #168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Any positive or negative comment about this proposal? |
def startAt[B >: seq.A, That](i: IndexO)(implicit bf: BuildFrom[C, B, That]): That = | ||
rotateLeft(i) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if this method should exists. When I have read its description, I though: but this is the same as rotateLeft
. Once I have checked the implementation, I see it really just calls it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OndrejSpanel I can definitely see your point.
The reason why, at the end, I went for the proposed duplication is that startAt
accepts an index, while rotateLeft
accepts a number of steps. Even if the result is the same, depending on how you see the circular sequence both can feel more natural.
Just as an example, startAt
plays nicely with reflectAt
another method you can find in my more complete and documented RingSeq
library, see https://scala-tessella.github.io/ring-seq/usage.html
I'm not sure if it's more natural as a view, like slices, an operation on indexes. The decisions to be taken are what to ingest here and in what form, and also whether it works better as a library. I would agree with the previous review that I see that this repo has a lower barrier to entry. |
Thank you @som-snytt for coming back to this issue. I deleted |
Why the |
It stands for When looking at the issue, consider that, in perspective, these other methods could have a circular alternative:
But if you think there is a better naming, I am happy to adopt it. |
I would like to add to
Seq
several operations, all related to considering aSeq
(or a subtype) a circular sequence.If you take a look at my library repo at https://github.com/scala-tessella/ring-seq, these are all the methods that I have in mind.
But I started here adding just three, since I need to get used to the intricacies of the Scala collections.
If this is of interest, I will have many more questions on how to correctly add other methods.