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.

Sign up for GitHub

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

Jump to bottom

Adding Floyd's Cycle Detection Algorithm #155

Closed
wants to merge 1 commit into from
Closed

Adding Floyd's Cycle Detection Algorithm #155

wants to merge 1 commit into from

Conversation

janhavikale06
Copy link
Contributor

Related Issue

#59

Proposed Changes

Added Floyd's Cycle Detection Algorithm

Additional Info

  • Any additional information or context

Checklist

  • [ * ] ✅ My code follows the code style of this project.
  • 📝 My change requires a change to the documentation.
  • [ * ] 🎀 I have updated the documentation accordingly.
  • [ * ] 👀 I have read the CONTRIBUTING document.
  • [ * ] ✨ I have added tests to cover my changes.
  • 🚩 All new and existing tests passed.
  • [ * ] 🌟 ed the repo
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Output Screenshots

Screenshot #1 Screenshot #2
Title goes here Title goes here
Image goes here Image goes here

@Lakhankumawat
Copy link
Owner

For contribution and more guidelines i would recommend you to go through this
tutorial video of the project here, thanks.

Hey @janhavikale06 , make sure your Pull Request is descriptive and complete the checklist .
I would recommend you to go through this tutorial video of the project here ,thanks.

Let me show you how to check the check list , there should be no space inside brackets like [x]
[x]

  • Checklist
  • Or you can just check them from outside without doing any of these
  • ✨ I have added tests to cover my changes.
  • 🚩 All new and existing tests passed.
  • 🌟 ed the repo
  1. Starr the repo if you haven't !
  2. Have you watched this video ?

@miraj0507
Copy link

Hey@

  1. Firstly I don't see the updates in the checklist, please [x] follow the format
  2. The code will give a runtime error, on the lines (p==NULL or q==NULL) as q can be the last node and q->next->next will give you a runtime error kindly fix the issues and soon your code might get merged

@janhavikale06
@Lakhankumawat first review done needs update.

@miraj0507 miraj0507 added the bug Something isn't working label Mar 2, 2022
@miraj0507
Copy link

Kindly add it to the right location this path is not expected

@janhavikale06
Copy link
Contributor Author

Hey@

  1. Firstly I don't see the updates in the checklist, please [x] follow the format
  2. The code will give a runtime error, on the lines (p==NULL or q==NULL) as q can be the last node and q->next->next will give you a runtime error kindly fix the issues and soon your code might get merged

@janhavikale06 @Lakhankumawat first review done needs update.
Floyd's Algo
Sorry but I'm not getting any error. Can you please check once again ?

@miraj0507
Copy link

It's because you are just using one particular test case , it's not generalised, run it for 1->2->3->4->5->null, create this linked list, any list of odd length having no cycle will give you error

n4i9kita
n4i9kita reviewed Mar 4, 2022
View reviewed changes
L-LinkedList/Floyd's Cycle Detection Algorithm #59.cpp
curr->next->next->next = new_node(4);
curr->next->next->next->next = new_node(5);
curr->next->next->next->next->next = new_node(6);
curr->next->next->next->next->next->next = loop_node;
Copy link
Contributor

@n4i9kita n4i9kita Mar 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janhavikale06 Don't you think it would be better to write the above line of code in a generalized way?
Rather than writing so many next's per instruction, just move the curr pointer ahead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it would be much better , I'll try do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @n4i9kita , can please give me a hint to create a linked list in generalized way which has loop node in it? If I'm asking user to give me nodes then how can I'll create a loop node in it?
I'm finding it difficult to do , can you please help me?

@janhavikale06 Don't you think it would be better to write the above line of code in a generalized way? Rather than writing so many next's per instruction, just move the curr pointer ahead.

Hey @n4i9kita , can please give me a hint to create a linked list in generalized way which has loop node in it? If I'm asking user to give me nodes then how can I'll create a loop node in it?
I'm finding it difficult to do , can you please help me?

L-LinkedList/Floyd's Cycle Detection Algorithm #59.cpp
@@ -0,0 +1,82 @@
/*Floyd's Cycle Detection Algorithm
Copy link
Contributor

@n4i9kita n4i9kita Mar 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the explanation of the algorithm will be provided in the readme section, it seems redundant here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay , got it.

L-LinkedList/Floyd's Cycle Detection Algorithm #59.cpp
node *new_node(int data)
{
node *curr ;
curr = (node*)malloc(sizeof(node));
Copy link
Contributor

@n4i9kita n4i9kita Mar 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to use new operator to allocate memory on heap, rather than malloc?

@Lakhankumawat Lakhankumawat added the g2m These are PR'S handled by Miraj label Mar 5, 2022
miraj0507
miraj0507 reviewed Mar 6, 2022
View reviewed changes
Copy link

@miraj0507 miraj0507 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest update, candidate didn’t-respond to previous proposed changes.

@janhavikale06
Copy link
Contributor Author

Latest update, candidate didn’t-respond to previous proposed changes.

Sorry I was having some personal issues that's why I didn't see all this responses. I will do all changes as soon as possible , sorry for my late response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@n4i9kita n4i9kita n4i9kita left review comments

@miraj0507 miraj0507 miraj0507 left review comments

@Lakhankumawat Lakhankumawat Awaiting requested review from Lakhankumawat

@iamgunjan iamgunjan Awaiting requested review from iamgunjan

@Poojaamangal Poojaamangal Awaiting requested review from Poojaamangal

Assignees
No one assigned
Labels
algorithm bug Something isn't working changes-pending enhancement New feature or request Feature-addition g2m These are PR'S handled by Miraj Level1 next review needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@janhavikale06 @Lakhankumawat @miraj0507 @n4i9kita