-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
refactor: RotateListRight.js and added tests #1101
Conversation
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.
Looks good. Ideally this should be a method of the SinglyLinkedList
class though; then you would also be able to leverage the get
method to obtain an array rather than having to do the tedious .head
. You could additionally add a constructor that takes a list to shorten your list creation.
@appgurueu Should I await your suggestion being implemented before merging? |
Not sure. The PR is definitely already an improvement in its current state, but the ideal solution would be what I have suggested. |
@10kartik Can you implement the suggested? |
Yeah @raklaptudirm, I'll give it a try... But I'm a bit doubtful about the suggested implementation.
As per my understanding @appgurueu, adding a method Let's say in a list of length 10, I want to rotate the list after the 4th node, so I'd simply pass the reference of the 5th node as Please correct me if I'm wrong or missing something here.
For this, I must add a constructor to the |
Valid point. The clean solution to that would be allowing "views" into sublists, comparable to Python or Go slices (although those work on array-based lists). Views would obviously again be linked lists and could thus have
Yes. |
Right! seems reasonable. Okay, so not sure how should I proceed with that 🤔. Should I shift the |
Yes, sounds good. Methods to create views / sublists can come later (or not at all). |
know more
Describe your change:
Checklist:
Example:
UserProfile.js
is allowed butuserprofile.js
,Userprofile.js
,user-Profile.js
,userProfile.js
are notFixes: #{$ISSUE_NO}
.