CODEWITHSUNDEEP
c++
Azazel
Azazel

Reputation: 25

C++ Linked list print error

I don't understand why the display() func show me only the first member of the list. I think I did a mess with pointers, but I can't understand where. I have compared this to other linked list source and it seem that the function is written in the-good-way.

#include "stdafx.h"
#include <string>
#include <iostream>
using namespace std;

struct Node
{
    int Data;
    Node * next;
};

void initNode(struct Node *head,int n);
void AddNode(int n,Node* head);
void display(Node* head);

int main()
{
    Node * head = new Node;

    initNode(head,5);
    display(head);

    AddNode(10,head);
    display(head);

    AddNode(15,head);
    display(head);

    cin.get();
    return 0;
}

void AddNode(int n,Node * head)
{
    Node * node = new Node;
    node->Data = n;
    node->next = NULL;

    Node * nextNode = head;
    while(nextNode)
    {
        if(nextNode->next == NULL)
        {
            nextNode->next = node;
        }
        nextNode = nextNode->next; 
    }
}

void display(Node * head)
{
    while(head)
    {
        cout << head->Data << " "<<endl;
        head = head->next;
    }
}

void initNode(struct Node *head,int n)
{
    head->Data = n;
    head->next = NULL;
}

Upvotes: 0

Views: 146

Answers (5)

Vlad from Moscow
Vlad from Moscow

Reputation: 310980

Function AddNode has an infinite loop.

void AddNode(int n,Node * head)
{
    Node * node = new Node;
    node->Data = n;
    node->next = NULL;

    Node * nextNode = head;
    while(nextNode)
    {
        if(nextNode->next == NULL)
        {
            nextNode->next = node;
        }
        nextNode = nextNode->next; 
    }
}

Let assume that you have only one element that is the head (after a call of initNode). And as the result head->next = NULL. So inside the body of the loop you make assignment

nextNode->next = node;

Now head->next is not equal to NULL. So after statement

nextNode = nextNode->next;

nextNode caontains the new element. As it is not equal to NULL then iteration of the loop will be repeated. Again for the new node its data member next is equal to NULL. And you add it to it itself.

Now you have no any element in the list that would have data member next equal to NULL. So you are unable to add new elements. The last element contains reference to itself. You could write the function the following way

void AddNode(int n,Node * head)
{
    Node * node = new Node;
    node->Data = n;
    node->next = NULL;

    Node * nextNode = head;
    while( nextNode -> next ) nextNode = nextNode->next;

    nextNode->next = node; 
}

But take into account that it is assumed that head is not equal to NULL. otherwise the function will be incorrect. I think that you should redesign you list.

Upvotes: 2

AAB
AAB

Reputation: 1653

Each time you add a node you traverse from head to the end of the list You can change it as follows

void AddNode(int n){
   Node *node=new Node;
   node->data=n;
   node->next=NULL; //head is global
   if(head==NULL){
      t=head=n;
   }
   else{
    t->next=n;  //t is global
    t=t->next;
   }
}

Upvotes: 0

Sean
Sean

Reputation: 62472

Your AddNode method is over-complicated. Do something like this to add to the front:

Node *AddNode(int n, Node *head)
{
  Node *newNode =  new Node;
  newNode->Data = n;
  newNode->next = head;

  return newNode;
}

Or to add to the end:

Node *AddNode(int n, Node *head)
{
  Node *newNode =  new Node;
  newNode->Data = n;
  newNode->next = NULL;

  if(head == NULL) return newNode;

  Node *current = head;

  while(current->Next != NULL)
  {
    current = current->Next;
  }

  current->Next = newNode;

  return head;
}

Doing AddNode this way you will not need initNode. Now you can just day:

Node *head = NULL;
head = AddNode(5, head);
head = AddNode(10, head);
head = AddNode(15, head);

display(head);

Also, you don't need to say struct Node in C++, it is only required in C.

Upvotes: 2

user2291878
user2291878

Reputation: 

Node * nextNode = head;
    while(nextNode)
    {
        if(nextNode->next == NULL)
        {
            nextNode->next = node;
        }
        nextNode = nextNode->next; 
    }

The problem is this block of code. When you find the end of the list (if nextNode->next == NULL) you need to break out of the loop. Try it with an example to convince yourself.

Upvotes: 0

overloading
overloading

Reputation: 1200

In your AddNode function add a break in the if block.

void AddNode(int n,Node * head)
{
    Node * node = new Node;
    node->Data = n;
    node->next = NULL;

    Node * nextNode = head;
    while(nextNode)
    {
        if(nextNode->next == NULL)
        {
            nextNode->next = node;
            break;
        }
        nextNode = nextNode->next; 
    }
}

Now it should add properly.

Upvotes: 1

Related Questions