Reputation: 35
I am attempting to create a simple doubly linked list to familiarize myself with pointers in c++. Each node contains an integer, a pointer to the next node, and a pointer to the previous node. When I attempt to output the values of each node in a linked list object, it prints values indefinitely.
My test code initializes a linked list with one node, and adds an additional 3 nodes.
When the PrintNodeVals() method is called, the while loop iterates indefinitely, outputting a constant stream of node values. When the for loop is used instead of the while loop, it prints the address of the head node once, then continually prints a second address, which is the first node attached using the addnode() method, and is the second node in the linked list overall.
The only explanation I can think of is that my code has somehow assigned the second nodes "next" pointer to the node itself, which would cause PrintNodeVals() while loop to always evaluate to true.
Any thoughts?
#include "LinkedList.h"
LinkedList::LinkedList(){
root = new Node();
}
//adds node to the end of the list
void LinkedList::addnode(){
Node newnode;
Node *conductor = root;
while(conductor->next != 0){
conductor = conductor->next; //(*conductor).next
}
conductor->next = &newnode; //makes the next field point to the new node
newnode.prev = conductor;
}
void LinkedList::PrintNodeVals(){
Node *conductor = root;
while(conductor != 0){
std::cout << conductor->val;
conductor = conductor->next;
}
/*
for(int i = 0; i < 10; i++){
std::cout << conductor << "\n";
conductor = conductor->next;
*/
}
}
//TEST CODE
#include <iostream>
#include "LinkedList.h"
using namespace std;
int main()
{
LinkedList linkle;
linkle.addnode();
linkle.addnode();
linkle.addnode();
linkle.ShowNodeVals();
return 0;
}
Upvotes: 2
Views: 1210
Reputation: 116
You should be allocating space when creating your newNode (which should be a pointer to node for the matter).
Remember, the model of double linked linear list should be connecting your node to the list (Which you are pointing to) and then connecting the list to the node.
Node *newNode = new Node();
newNode->data = element //Set your element in your node here
Node *conductor = root;
while (conductor->next != nullptr) {
conductor = conductor->next;
}
//So you will be connecting your new element like:
newNode->next = nullptr; //Connect node in last position
newNode->prev = conductor; //Connect node to previous position which should be stored by conductor doing that loop
//Then connect the list to the new node
conductor->next = newNode;
Also, you might want to check your constructor and make sure the first element in the list (which is created there) is pointing to NULL in both sides.
Remember this only works if you are adding elements to the last position of your list, if you are inserting in positions then you should be taking in count various cases to make some really juicy and nice code!
Hope this helps!
P.D: If you need any more help just send a message. :)
Upvotes: 1
Reputation: 55725
The problem is that you are storing a pointer to a local variable in the list:
Node newnode;
// ...
conductor->next = &newnode;
The newnode
is destroyed at the end of the block and the pointer becomes invalid. You should probably dynamically allocate the new node or use std::list
instead of your own list class.
Upvotes: 1