Finding cycle in singly linked list

Question

Finding a cycle in singly linked list and find the node from where cycle starts. I have seen use of two pointers( generally slow and fast) to find the cycle but I have written this code and it seems to be working fine. My question is, is there something my code is missing out on, while finding the cycle in singly linked list.

Node* find_cycle_node(Node *head){
Node *p=head;
     Node *q;
     while(p->next!=null)
    { 
              q=p->next;
        while(q->next!=null)
          { 
                     if(p==q) return q; //Node repeated i.e cycle
                     else (q=q->next;)
           }
                p=p->next;
      }
    return null; // no cycle detected
}

Answer 1

Your inner loop will not terminate if there is cycle which is a couple nodes down the handle, e.g., it will be an infinite loop for something like this:

1 -> 2 -> 3 -> 4 -> 5
          ^         |
          |         |
          +---------+

Answer 2

How about this ?

struct Node_
{
    int ix ;
    struct Node_* next ;
} ;
typedef struct Node_ NODE ;
NODE *head = NULL ;

int main()
{
    NODE* n1 ;
    n1 = (NODE*) malloc(sizeof(NODE)) ;
    n1->ix = 0 ;
    n1->next = NULL ;
    head = n1 ;
    NODE* n2 ;
    n2 = (NODE*) malloc(sizeof(NODE)) ;
    n2->ix = 1 ;
    n2->next = NULL ;
    n1->next = n2 ;
    NODE* n3 ;
    n3 = (NODE*) malloc(sizeof(NODE)) ;
    n3->ix = 2 ;
    n3->next = NULL ;
    n2->next = n3 ;
    NODE* n4 ;
    n4 = (NODE*) malloc(sizeof(NODE)) ;
    n4->ix = 3 ;
    n4->next = n2 ;
    n3->next = n4 ;

    unordered_map<NODE*,int> hashx ;
    int idx ;
    NODE* p = head ;
    while(p != NULL)
    {
        hashx[p] += 1 ;
        if(hashx[p] >= 2)
        {
            printf("node p (%d) recycle !!\n",p->ix);
            break ;
        }
        p = p->next ;
    }
    printf("done \n") ;
} //main

Answer 3

is there something my code is missing out on

return; // no cycle detected

This line looks pretty bad, it should be changed to s.th. like

return NULL; // no cycle detected

Answer 4

To me your inner loop condition appears to be ambiguous. You are analysing if (p==q) where q is p-> next. this means that the node p previously considered didn't haD A CYCLE. So to me your inner loop wil never terminate.

you must consider this:-

#include <iostream>
using namespace std;
class Node{
    public:
        int data;
        Node * next;
        Node(int x){
            data = x;
            next = NULL;
        }
        Node(int x, Node * y){
            data = x; 
            next = y;
        }
};
class linkedList{
    Node *head;
    public:
        linkedList(){
            head = NULL;
        }
        void addNode(int value){
            Node *p;
            if(head == NULL)
                head = new Node (value, NULL);
            else{
                p = head;
                while(p->next !=NULL)
                    p=p->next;
                    p->next = new Node (value, NULL);
            }
        }
        void print(){
            Node * p;
            p = head;
            while(p != NULL){
                cout << p->data;
                p = p->next;
            }
        }
        int findCycle(){
            Node *p, *start, *q;
            p = head;
            while(p != NULL){
                q = p->next;
                while (q != NULL ){
                    if(p->data == q->data)
                         return q->data;
                    else
                         q = q->next;
                }
                p = p->next;
            }
            return 0;
        }
};
int main(){
    linkedList l1;
    l1.addNode(1);
    l1.addNode(2);
    l1.addNode(3);
    l1.addNode(4);
    l1.addNode(5);
    l1.addNode(3);
    int node = l1.findCycle();
    cout<<node;
    return 0;
}

What do you say people about this code.

Answer 5

void LinkListOps::createCycledListAndFindACycleNode()

{ // build a list with a cycle in it

ZNODE* head = new ZNODE(0);
ZNODE* current = head;
ZNODE* cycle = 0;

for (int i = 1; i < 8; i++)
{
    current->_next = new ZNODE(i);
    current = current->_next;

    if (i == 6)
        cycle = current;

    if (i == 7)
        current->_next = cycle;
}

// verify that there is a cycle
ZNODE* slow = head;
ZNODE* fast = head;
ZNODE* cycleNode = 0;

while (slow && fast && fast->_next)
{
    slow = slow->_next;
    fast = fast->_next->_next;

    if (slow == fast)
    {
        cycleNode = slow;
        break;
    }

}

if (cycleNode == 0)
{
    printf("No cycle\n");
    return;
}

// finally find a cycle node which will be p2
ZNODE* p1 = head;
ZNODE* p2 = cycleNode;

while (1)
{
    for (p2 = cycleNode; p2->_next != cycleNode; p2 = p2->_next)
    {
        if (p2 == p1)
            break;          
    }

    if (p2 == p1)
        break;

    p1 = p1->_next;
}

printf("%d\n", p2->_i);

}

Answer 6

You can quickly find out if there is a loop in a linked list by doing the following:

ptr = head;
current = nullptr;
if (!ptr) {
  current = ptr->next;
  while(current && current!=ptr) {
    current = current->next;
    if (current) {
      current = current->next;
    }
    ptr = ptr->next;
  }
}

At the end of this, if current is not null, then a loop was found, and current will be somewhere inside of it. This works by iterating current twice as fast through the list as ptr, and will always find a loop, if any exists, before ptr has looped once.