Linked List, insert at the end C++

Question

I was writing a simple function to insert at the end of a linked list on C++, but finally it only shows the first data. I can't figure what's wrong. This is the function:

void InsertAtEnd (node* &firstNode, string name){

        node* temp=firstNode;

        while(temp!=NULL) temp=temp->next;

            temp = new node;
        temp->data=name;
        temp->next=NULL;

        if(firstNode==NULL) firstNode=temp;

}

Answer 1

#include <bits/stdc++.h>

using namespace std;

class Node
{
public:
    int data;
    Node *next;
};

void append(Node *first, int n)
{
    Node *foo = new Node();
    foo->data = n;
    foo->next = NULL;

    if (first == NULL)
    {
        first = foo;
    }
    else
    {
        Node *last = first;
        while (last->next)
            last = last->next;
        last->next = foo;
    }
}

void printList(Node *first)
{
    while (first->next != NULL)
    {
        first = first->next;
        cout << first->data << ' ';
    }
}

int main()
{
    Node *node = new Node();
    append(node, 4);
    append(node, 10);
    append(node, 7);

    printList(node);

    return 0;
}

Output: 4 10 7

Answer 2

You've made two fundamental mistakes:

1. As you scroll through the list, you roll off the last element and start constructing in the void behind it. Finding the first NULL past the last element is useless. You must find the last element itself (one that has its 'next' equal NULL). Iterate over temp->next, not temp.

2. If you want to append the element at the end, you must overwrite the last pointer's NULL with its address. Instead, you write the new element at the beginning of the list.

void InsertAtEnd (node* &firstNode, string name)
{
   node* newnode = new node;
   newnode->data=name;
   newnode->next=NULL;

   if(firstNode == NULL)
   {
        firstNode=newnode;
   }
   else
   {
        node* last=firstNode;
        while(last->next != NULL) last=last->next;
        last->next = newnode;
   }
}

Note, this gets a bit neater if you make sure never to feed NULL but have all lists always initialized with at least one element. Also, inserting at the beginning of list is much easier than appending at the end: newnode->next=firstNode; firstNode=newnode.

Answer 3

void InsertAtEnd (node* &firstNode, string name){

        node* temp=firstNode;

        while(temp && temp->next!=NULL) temp=temp->next;

       node * temp1 = new node;
        temp1->data=name;
        temp1->next=NULL;
       if(temp==NULL)
           firstNode=temp1;
       else
           temp->next= temp1;


}

while loop will return at temp==null in your code instead you need to return last node pointer from while loop like this

while(temp && temp->next!=NULL) temp=temp->next;

and assign a new node to next pointer of the returned temp node will add the data to the tail of linked list.

Answer 4

void addlast ( int a)
{

    node* temp = new node;
    temp->data = a;
    temp->next = NULL;
    temp->prev=NULL;
    if(count == maxnum)
    { 
        top = temp;
        count++; 
    } 
    else
    { 
        node* last = top;
        while(last->next)
            last=last->next;
        last->next = temp;
    }
}

Answer 5

You can use this code:

void  insertAtEnd(Node* firstNode, string name)
{
    Node* newn = new Node;              //create new  node
    while( firstNode->next != NULL )    //find the last element in yur list
        firstNode = firstNode->next;    //he is the one that points to NULL 
    firstNode->next = newn;             //make it to point to the new element
    newn->next = NULL;      //make your new element to be the last (NULL)
    newn->data = name;      //assign data.
}

Answer 6

If you don't want to use reference pointer, you could use pointer to pointer. My complete code goes like below:

void insertAtEnd(struct node **p,int new_data)
{
    struct node *new_node=(struct node *)malloc(sizeof(struct node));
    new_node->data=new_data;
    new_node->next=NULL;
    if((*p)==NULL)//if list is empty
    {
        *p=new_node;
        return;
    }
    struct node* last=*p;//initailly points to the 1st node
    while((last)->next != NULL)//traverse till the last node
        last=last->next;
    last->next=new_node;
}
void printlist(struct node *node)
{
    while(node != NULL);
    {
        printf("%d->",node->data);
        node=node->next;
    }
}
int main()
{
    struct node *root=NULL;
    insertAtEnd(&root,1);
    insertAtEnd(&root,2);
    insertAtEnd(&root,3);
    insertAtEnd(&root,4);
    insertAtEnd(&root,5);
    printlist(root);
return 0;
}    

Understanding the need of the below two variables is key to understanding the problem:

1. struct node **p: Because we need to link it from the root node created in the main.
2. struct node* last: Because if not used, the original content will be changed with the contents of the next node inside the while loop. In the end only 2 elements will be printed, the last 2 nodes, which is not desired.

Answer 7

The problem is that you are replacing the head of the linked list with the new element, and in the process losing the reference to the actual list.

To insert at the end, you want to change the while condition to:

while(temp->next != null)

After the loop, temp will point to the last element in the list. Then create a new node:

node* newNode = new node;
newNode->data = name;
newNode->next = NULL;

Then change temps next to this new node:

temp->next = newNode;

You also do not need to pass firstNode as a reference, unless you want NULL to be treated as a linked list with length 0. In that case, you will need to significantly modify your method so it can handle the case where firstNode is NULL separately, as in that case you cannot evaluate firstNode->next without a segmentation fault.

Answer 8

What you wrote is:

• if firstNode is null, it's replaced with the single node temp which has no next node (and nobody's next is temp)

• Else, if firstNode is not null, nothing happens, except that the temp node is allocated and leaked.

Below is a more correct code:

void insertAtEnd(node* &first, string name) {
    // create node
    node* temp = new node;
    temp->data = name;
    temp->next = NULL;

    if(!first) { // empty list becomes the new node
        first = temp;
        return;
    } else { // find last and link the new node
        node* last = first;
        while(last->next) last=last->next;
        last->next = temp;
    }
}

Also, I would suggest adding a constructor to node:

struct node {
    std::string data;
    node* next;
    node(const std::string & val, node* n = 0) : data(val), next(n) {}
    node(node* n = 0) : next(n) {}
};

Which enables you to create the temp node like this:

node* temp = new node(name);

Answer 9

The last element in your list never has it's next pointer set to the new element in the list.