Skip to content
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

Added CircularDoublyLinkedList.js alongwith it's test code #1414

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

debnath003
Copy link

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.

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new JavaScript files are placed inside an existing directory.
  • All filenames should use the UpperCamelCase (PascalCase) style. There should be no spaces in filenames.
    Example:UserProfile.js is allowed but userprofile.js,Userprofile.js,user-Profile.js,userProfile.js are not
  • All new algorithms have a URL in their comments that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

@debnath003 debnath003 changed the title Added CircularDoublyLinnkedList.js alongwith it's test code Added CircularDoublyLinkedList.js alongwith it's test code Oct 3, 2023
@debnath003
Copy link
Author

@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

Copy link
Collaborator

@appgurueu appgurueu left a 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.

@debnath003
Copy link
Author

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

@debnath003
Copy link
Author

[...] 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! 😁

@debnath003
Copy link
Author

debnath003 commented Oct 3, 2023

Did you check that out @appgurueu ? Please help to merge this PR so that it can be admitted as "hacktoberfest-accepted"

@debnath003
Copy link
Author

@appgurueu awaiting your review

@debnath003
Copy link
Author

It showed one error, can I edit that @appgurueu ?

Copy link
Collaborator

@appgurueu appgurueu left a 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.

@debnath003
Copy link
Author

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. :)

@debnath003
Copy link
Author

Could you please merge this one so that it can be eligible for hacktoberfest? @appgurueu

@debnath003
Copy link
Author

@appgurueu @raklaptudirm please check my edits and merge my PR, awaiting your review for a long time!

@debnath003
Copy link
Author

@appgurueu @raklaptudirm please check my PR and approve the review for merging the PR for hacktoberfest

raklaptudirm
raklaptudirm previously approved these changes Oct 6, 2023
@debnath003
Copy link
Author

@appgurueu please approve and merge this PR! And give hacktoberfest-accepted label so that it can be eligible for hacktoberfest! Thank you

@raklaptudirm
Copy link
Member

Please stop pinging the maintainers, we'll review it when we find the time.

@debnath003
Copy link
Author

@appgurueu please approve the PR and add hacktoberfest-accepted label os that it can be counted in hacktoberfest. Waiting for the review!

Copy link
Collaborator

@appgurueu appgurueu left a 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.

@debnath003
Copy link
Author

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.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants