Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 5.8k
implemented CycleDetectionII code in LinkedList#1482
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Akshay73c commented Oct 8, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
appgurueu left a comment
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 is still messy (mostly formatting-wise, the
lengthCyclefunction should be defined before use for better readability, IMO). Please add comments, and give better variable names (i.e. I assumefandsarefastandslow; why do you need them when you already havefastandslow?). - As for the tests, remove the 3 redundant comments there which just repeat test case names.
Akshay73c commented Oct 12, 2023
Dear maintainers,
Let me know! And thanks again for the review :) I wanted to reach out to let you know that while attempting my first contribution to this open source project, I ran into some issues with git push. (That's why so many commits) After doing some research, I was able to resolve the issues and successfully contribute my changes. |
Akshay73c commented Dec 2, 2023
Hey maintainers, I'd be really glad if you could review this ;) |
appgurueu left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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.
Of course! This seems to work, but there is definitely dead / unnecessary code, and why the code is doing what it is doing is not exactly clear. Also some minor stylistic remarks.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Akshay73c commented Dec 11, 2023
Dear maintainers, |
Akshay73c commented Dec 17, 2023
Please let me know if there's anything more that needs to be addressed. Fingers crossed for the merge ;) |
Uh oh!
There was an error while loading. Please reload this page.
appgurueu commented Jan 3, 2024
Sorry for the long delay. I simplified the code a bit and renamed some variables. I hope you're fine with this, otherwise feel free to complain :) LGTM! |
Akshay73c commented Jan 4, 2024
LGTM too! Thanks for simplifying things, and no complaints here :)) |
Hey maintainers,
I've implemented the code for getting the starting node of cycle in a linked list which i think will be useful for understanding of cycle-type-questions.
I've also added the tests to check the correctness.
It'll be my first open source contribution and also in Hacktoberfest. Hoping a success :)
Changes:
Checklist:
I've read the CONTRIBUTING.md file and also checked the testcases using npm as asked to.
This pr is all my own work.