joss
joss

Reputation: 765

Pointer changing automatically after function call for simple linkedlist C++

I am a beginner in C++, I was implementing simple linked list in C++. While I try to add a second element to list, the pointer for next element in the head is changing

int main()
{
    LinkedList ll;
    string test;
    test="we";
    ll.add("we");
    ll.add("are");
    cout << "ok working" << endl;
    return 0;
}

class LinkedList
{
    public:
        LinkedList();
        void add(string data);
    protected:
    private:
        class node{
        private:
            string data;
        public:
            node *next;
            node(string data,node *next);
            node(string data);
            node();
        };

        node *head;
};

LinkedList::LinkedList()
{
    head=NULL;
    cout<<"ok";
}


void LinkedList::add(string data){
    if(!head){
        node tmphead(data,NULL);
        this->head=&tmphead;
    }else{
        node *temp;
        temp=head;
        while(temp->next){
            temp=temp->next;
        }
        node newnode(data);
        temp->next=&newnode;
    }
}
LinkedList::node::node(string data,node *next){
    LinkedList::node::data=data;
    LinkedList::node::next=next;
cout<<"New node created with : "+data<< endl;

}
LinkedList::node::node(){
    LinkedList::node::data="";
    LinkedList::node::next=NULL;
    //LinkedList::node::
//this->data="";
//this->next=NULL;
}
LinkedList::node::node(string data){
LinkedList::node::data=data;
LinkedList::node::next=NULL;
}

when adding "we", head-> next is 0x0. Once the control goes to the add function again for "are", the head->next is 0x3. It is getting changes automatically.

Any help is really appreciated.

Upvotes: 0

Views: 223

Answers (2)

rpaulin56
rpaulin56

Reputation: 476

The problem is you are building a dynamic structure without using dynamic memory. The lifetime of the variables tmphead and newnode is limited to the add() call, so their addresses become invalid on exit. Please review new and delete sections in some tutorial

Upvotes: 2

Tom&#225;š Zato
Tom&#225;š Zato

Reputation: 53358

You're assigning pointer to local variable to head. That's really bad.

if(!head){
    node tmphead(data,NULL);
    this->head=&tmphead;
} // After this bracket, tmphead was dealocated

When you do this kinda stuff, anything can happen since you're accessing memory that is not allocated to you any more.

Same here:

    node newnode(data);
    temp->next=&newnode;
    // after function exits, `temp->next` points to deallocated memory

Instead, use new in both cases:

void LinkedList::add(string data){
    if(!head){
        this->head = new node(data, nullptr);
    }else{
        node* temp;
        temp=head;
        while(temp->next){
            temp = temp->next;
        }
        temp->next = new node(data);
    }
}

Also, remember to create some way to delete everything you created using new.

For example:

LinkedList::~LinkedList() {
    delete head;
}
node::~node() {
    // WARNING: this is recursive
    // that's not bad but it's important to be aware of it
    if(next != nullptr)
        delete next;
}

You don't need to delete the string data - it's destructor is called automatically when node is deleted.

Upvotes: 1

Related Questions