CODEWITHSUNDEEP
c++constructorlinked-list
user2881037
user2881037

Reputation: 23

Singly Linked List - C++ - Unexplainable run-time error

I wrote the following code, to just create and insert nodes (integer data) into an SLL, in C++.

#include <stdio.h>

class Node
{
    public:
        int data;
        Node * next;
        Node * first;
        Node() {}

        void insert(int dat)
        {
            Node * newnode = new Node();
            newnode->data=dat;
            newnode->next=NULL;
            if(first==NULL)
            {
                first=newnode;
            }
            else
            {
                Node *temp=first;
                while(temp->next!=NULL)
                { temp=temp->next; }
                temp->next=newnode;
            }
        }
};

int main()    
{
    Node * a=new Node();
    a->insert(12);
    return 0;
}

At first, I tried overriding the Node constructor to Node(int dat) and in that I tried doing initialization of every new node (data=dat, next=NULL) that I create in insert. Insert would be called with a "dat" value from main and it would call the overloaded Node constructor to initialize data to dat and next to NULL. That led to my program crashing.

So I took out both default and overloaded constructor and did the initialization of each new element in the insert itself. My program works fine. However, even adding the default constructor (as shown in Line 10 of the code) my program crashes. Can anyone tell me why this is happening in both cases?

Thanks.

Upvotes: 0

Views: 440

Answers (3)

kunal
kunal

Reputation: 966

initialize member variables in constructor like this

Node() {
             data =0;
             first = NULL;
             next = NULL;
}

Upvotes: 0

Benjamin Lindley
Benjamin Lindley

Reputation: 103693

Your default constructor leaves the data members uninitialized. So this line:

Node * a=new Node();

creates a Node with uninitialized members, leading to problems when you try to add a node. When you remove your default constructor, the above line (due to the parenteses in new Node() combined with the fact that the class has no user defined constructors) results in a value initialization of all the members, so the pointers get initialized to NULL, and you get the expected behavior.

If you had left the parentheses out:

Node * a = new Node;

The data members would be uninitialized, just as when you had a do nothing default constructor.

The correct solution is to fix your default constructor to explicitly initialize all members.

Node() :data(0), next(nullptr), first(nullptr) {}

Upvotes: 1

Tony Delroy
Tony Delroy

Reputation: 106086

Node * a=new Node();

Creates a new node, running its default constructor...

Node() {}

...which doesn't do much - not even initialise data, next or first.

Consequently, when you call...

a->insert(12);

...and it tries...

if(first==NULL)

...it's reading from uninitialised memory, resulting in undefined behaviour. On one run the newly-created object might happen to have a 0 in there, another time it might not, some other time it might just crash trying to read the value. You go on to do other worse things with the uninitialised data.

More generally, when you have such problems I recommend you put std::cerr << xyz << '\n'; statements in to dump out some relevant variables, or trace through in the debugger - you might have seen that the variables had garbage values, then could have started to investigate why, which might have yielded a more focused question here, or perhaps led you to an existing answer....

Upvotes: 0

Related Questions