-3

I am new to linked list..My simple code is to create linked list and insert nodes at the end and traverse it..
My problems are-
1)-Every time insert function is called,head pointer gets null
2)-not working right while going in show function..

Please help..Thanks in advance

#include<iostream>
#include<malloc.h>
using namespace std;

struct linkedList
{
    int value;
    linkedList *next;
};
linkedList* head = NULL;
void insert(linkedList* head, int data)
{

    linkedList *ptr;
    linkedList *node;
    node = (linkedList*) malloc(sizeof(struct linkedList));
    node->value = data;
    node->next = NULL;
    if (head == NULL)
    {

        head = node;

    }
    else
    {
        ptr = head;
        while (ptr != NULL)
        {
            ptr = ptr->next;
        }
        ptr = node;
    }
}
void show(struct linkedList *head)
{

    struct linkedList *ptr;
    ptr = head;

    while (ptr != NULL)
    {
        cout << ptr->value << endl;
        ptr = ptr->next;
    }
}
int main()
{

    int size = 5;

    int array[size];
    for (int i = 0; i < 5; i++)
    {
        cout << "Enter value" << endl;
        cin >> array[i];

        insert(head, array[i]);
    }

    show(head);

}
  • 2
    Learn about the difference between passing arguments *by value* and *by reference*. – Some programmer dude Sep 16 '16 at 20:19
  • Welcome to Stack Overflow! It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: **[How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/)** – Khalil Khalaf Sep 16 '16 at 20:24
  • 1
    Looks like you're learning `C` instead of `C++`. What's that `malloc` doing there (instead of `new`)? – PaulMcKenzie Sep 16 '16 at 20:24
  • @JoachiPileborg can you write the code please..that would me a real help – Harshit Jain Sep 16 '16 at 20:26
  • 3
    [Find a good beginners book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list), it will tell you all you need to know. – Some programmer dude Sep 16 '16 at 20:34

1 Answers1

0

In your insert() function:

  • when head is NULL, you are assigning the new node to the local head parameter, which is not updating the caller's head variable. That is why your global head variable is always NULL. This is because you are passing the head parameter by value, so you are assigning the new node to a copy, not the original. You need to pass the parameter by reference/pointer instead.

  • when head is not NULL, you are not traversing the nodes correctly to find the tail node, so ptr is always NULL after the traversal. You are not setting the next field of the tail node at all.

Also, your main() is leaking the allocated nodes.

Try something more like this instead:

#include <iostream>

struct linkedNode
{
    int value;
    linkedNode *next;
};

void insertValue(linkedNode* &head, int data)
{
    linkedNode *node = new linkedNode;
    node->value = data;
    node->next = NULL;

    if (!head)
    {
        head = node;
    }
    else
    {
        linkedNode *ptr = head;
        while (ptr->next)
        {
            ptr = ptr->next;
        }
        ptr->next = node;
    }
}

void showValues(linkedNode *head)
{
    linkedNode *ptr = head;
    while (ptr)
    {
        std::cout << ptr->value << std::endl;
        ptr = ptr->next;
    }
}

void freeValues(linkedNode* &head)
{
    linkedNode *ptr = head;
    head = NULL;

    while (ptr)
    {
        linkedNode *next = ptr->next;
        delete ptr;
        ptr = next;
    }
}

int main()
{
    linkedNode* mylist = NULL;

    for (int i = 0; i < 5; ++i)
    {
        std::cout << "Enter value" << std::endl;

        int value;
        if (std::cin >> value)
            insertValue(mylist, value);
    }

    showValues(mylist);
    freeValues(mylist);

    return 0;
}

That being said, if you keep track of the tail node in the list, inserts at the end would be much faster and efficient since you would not need to traverse the list at all:

#include <iostream>

struct linkedNode
{
    int value;
    linkedNode *next;

    linkedNode(int data)
        value(data), next(NULL)
    {
    }
};

struct linkedList
{
    linkedNode *head;
    linkedNode *tail;

    linkedList()
        : head(NULL), tail(NULL)
    {
    }

    ~linkedList()
    {
        linkedNode *ptr = head;
        while (ptr)
        {
            linkedNode *next = ptr->next;
            delete ptr;
            ptr = next;
        }
    }

    void insert(int data)
    {
        linkedNode *node = new linkedNode(data);

        if (!head)
            head = node;

        if (tail)
            tail->next = node;
        tail = node;
    }

    void showValues()
    {
        linkedNode *ptr = head;
        while (ptr)
        {
            std::cout << ptr->value << std::endl;
            ptr = ptr->next;
        }
    }
};

int main()
{
    linkedList mylist;

    for (int i = 0; i < 5; ++i)
    {
        std::cout << "Enter value" << std::endl;

        int value;
        if (std::cin >> value)
            mylist.insert(value);
    }

    mylist.showValues();

    return 0;
}

In which case, you could just throw all of this away and use the standard std::list class instead:

#include <iostream>
#include <list>
#include <algorithm>

void showValue(int value)
{
    std::cout << value << std::endl;
}

void showValues(const std::list<int> &values)
{
    std::for_each(values.begin(), values.end(), showValue);

    /* or, if you are using C++11:

    std::for_each(values.begin(), values.end(),
        [](int value){ std::cout << value << std::endl; }
    );
    */
}

int main()
{
    std::list<int> mylist;

    for (int i = 0; i < 5; ++i)
    {
        std::cout << "Enter value" << std::endl;

        int value;
        if (std::cin >> value)
            mylist.push_back(value);
    }

    showValues(mylist);

    return 0;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770