Reputation: 765
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
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
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