-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Added CircularDoublyLinkedList.js alongwith it's test code #1414
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
Conversation
@appgurueu @raklaptudirm please check this review, I have added one important algorithm which was missing in the linked list directory, please approve this PR and merge the PR so that it can get eligible for hacktoberfest
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.
Interface-wise, which advantages does this have over our existing...
- circular singly linked list?
- doubly linked list?
(what is a usecase where you need to use this rather than our existing data structures?)
Code-wise:
toString
and print
should be deduplicated (ideally there should be a toArray
method or a method which returns an iterator).
insert
seems to handle prepending as a special case, but not appending? Why? Why does prepending not get its own method like append
? Why does removal not get remove first / remove last methods? I think for consistency, the API should either be insertion / removal with indices (where i = 0 and i = length - 1 should be efficiently implemented special cases), or there should be six methods - {insert, remove} times {First, Last, ByIndex}.
This also needs JSDoc comments.
Firstly, the circular doubly linked list will combine some advantages of a circular linked list and a doubly linked list, the circular doubly linked list can go bidirectional, as each node has next and previous pointers, secondly, unlike singly linked lists, circular doubly linked lists naturally support wraparound navigation. This means that if you reach the last node while traversing forward, you will automatically loop back to the first node, and vice versa.
One use case that I want to mention is specifically targeted at students:- such as in DSA. Some algorithms and data structures may benefit from the circular nature of doubly linked lists, such as certain graph algorithms, where you need to traverse a graph in a cyclic manner. In those problem cases, we can apply circular doubly linked lists' algorithm.
And regarding code, yeah I will replace that toString
and print
method to rectify the methods toArray
and an iterator function. And regarding insert
one, I am again rechecking the code, and editing out the flaws.
And where to put the JSDoc comments? @appgurueu
[...] Some algorithms and data structures may benefit from the circular nature of doubly linked lists, such as certain graph algorithms, where you need to traverse a graph in a cyclic manner. In those problem cases, we can apply circular doubly linked lists' algorithm.
This is still very vague. Can you give a concrete code example which significantly benefits (in conciseness / time complexity / space complexity) from this implementation of a circular doubly linked list? Are you sure that the interface you have exposed suffices for these use cases? Are you sure that the interfaces of the doubly linked list or the (circular) singly linked list would be significantly less suitable?
And where to put the JSDoc comments?
Mostly on the methods, though there should also be one on the class. You can find an example here (but please try to make the comments less redundant vs. that example).
As of now I don't know any type of example, though I believe this code will take less time complexity, and we always know, no code in data structures is concise until we try to do that, although it will take some time! 😅
My use case is for students and DSA enthusiasts who want to get the resource and implement it in their projects, I believe, this code can fulfill the requirements! :)
And I have updated the code, could you please check it again @appgurueu? I want to contribute in hacktoberfest since it's my first time participating and contributing, so please approve this PR, it will be a big encouragement for me! 😁
Did you check that out @appgurueu ? Please help to merge this PR so that it can be admitted as "hacktoberfest-accepted"
@appgurueu awaiting your review
It showed one error, can I edit that @appgurueu ?
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.
The code looks fine, though
- Inserting/removing at the head takes a fast path, but not inserting/removing at the tail. Why?
- Still, my main concern is that given the current interface, this is practically redundant in possible usage with our existing doubly linked list; basically, it's a doubly linked list which, except for using
null
as a sentinel, uses a circular reference to the head as a sentinel, which makes wraparound more convenient indeed (but not by much); this doesn't seem to be reflected / leveraged in the API at all though. DSA learners should not be blindly learning random algorithms and data structures without knowing their applications.
The code looks fine, though
- Inserting/removing at the head takes a fast path, but not inserting/removing at the tail. Why?
- Still, my main concern is that given the current interface, this is practically redundant in possible usage with our existing doubly linked list; basically, it's a doubly linked list which, except for using
null
as a sentinel, uses a circular reference to the head as a sentinel, which makes wraparound more convenient indeed (but not by much); this doesn't seem to be reflected / leveraged in the API at all though. DSA learners should not be blindly learning random algorithms and data structures without knowing their applications.
Regarding the first one, I have to check again or rectify the code again, and regarding the second one, I know this is cumbersome and a bit redundant also, but given the usage of DSA in academics, or considering the importance of all algorithms, I thought it will be a nice touch to add this algorithm also. :)
Could you please merge this one so that it can be eligible for hacktoberfest? @appgurueu
@appgurueu @raklaptudirm please check my edits and merge my PR, awaiting your review for a long time!
@appgurueu @raklaptudirm please check my PR and approve the review for merging the PR for hacktoberfest
@appgurueu please approve and merge this PR! And give hacktoberfest-accepted label so that it can be eligible for hacktoberfest! Thank you
Please stop pinging the maintainers, we'll review it when we find the time.
@appgurueu please approve the PR and add hacktoberfest-accepted label os that it can be counted in hacktoberfest. Waiting for the review!
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.
For consistency, insert
should be called insertAt
, and append
should be a special case of insertAt
for position === this.length
.
Still, my main issue with this persists: It is basically just a doubly linked list interface-wise: With the interface given, I see no way to leverage the circular or "wraparound" property, and even if it was possible to use the "wraparound", I would question that this property alone is worth duplicating much linked list code - implementation-wise, this is practically just a linked list which uses a pointer to the head instead of a null pointer as a sentinel currently.
For consistency,
insert
should be calledinsertAt
, andappend
should be a special case ofinsertAt
forposition === this.length
.Still, my main issue with this persists: It is basically just a doubly linked list interface-wise: With the interface given, I see no way to leverage the circular or "wraparound" property, and even if it was possible to use the "wraparound", I would question that this property alone is worth duplicating much linked list code - implementation-wise, this is practically just a linked list which uses a pointer to the head instead of a null pointer as a sentinel currently.
Actually, this is my main motive, to update the mechanism of a linked list to some code that will point to the head, to make it circular. My code is largely made for DSA students, who will find an easy solution for the circular doubly linked list, that's why I Implemented here, generally in our university syllabus, this topic is there, but to help students find a code based on this algorithm, that's why I included it here. @appgurueu
Open in Gitpod know more
Describe your change:
Added the code for Circular Doubly Linked List in the "Data Structures" directory, along with it's test code in the "test" directory.
Checklist:
Example:
UserProfile.js
is allowed butuserprofile.js
,Userprofile.js
,user-Profile.js
,userProfile.js
are notFixes: #{$ISSUE_NO}
.