Reputation: 1
all! I don't know if 'overridden' is the right word here. In my programming class, I have to create a circular list, such that each node node contains a pointer pointing to the next node and the final node points to the first node. In addition, there is a tail node that points to the last node added (its null before any nodes are added).
I cannot populate my list (called a ring) because every time I call the Ring::Insert(const int& d) function, which inserts a single node, and it gets to the line "RingNode newNode(d);", the new RingNode object overwrites the previous RingNode object that was created when I last called the Ring::Insert(const int& d) function. Obviously, I don't want this because it messes up my list. How do I make it so every time the function creates a brand new RingNode object it doesn't interfere with the previous RingNode objects?
Source code from my header file, just in case:
class RingNode {
public:
RingNode(const int& i=0 ): data(i), next(NULL){}
private:
int data; /* ID of player */
RingNode* next;
friend class Ring;
And here is the function in question
RingNode* Ring::Insert(const int& d){
RingNode newNode(d); //This line overwrites previous RingNode objects
RingNode* refNode = &newNode; //Probably bad form, but that's not my main concern right now
if (tail==null){
tail = refNode;
newNode.next = refNode;
return refNode;
}
newNode.next = (*GetTail()).next;
(*GetTail()).next = refNode;
tail = refNode;
return refNode;
}
So, for example, if I execute the following snippet in my main...
Ring theRing;
theRing.Insert(5);
theRing.Insert(2);
theRing.Insert(7);
If I debug my project I can see that theRing contains only one RingNode, first it's the 5 RingNode, then the 2 RingNode overwrites it, then the 7 RingNode overwrites that. Thanks for reading and double thanks for your replies!
EDIT: I replaced
RingNode newNode(d);
RingNode* refNode = &newNode;
with
RingNode *newNode = new RingNode(d);
tweaked the rest of the code, and it's working properly. Thanks so much for the help, guys! Very informative and best of all I now understand why it was messing up.
Upvotes: 0
Views: 1124
Reputation: 399813
You can't add an object allocated as an "auto" variable, i.e. on the stack, like that. You need to do RingNode *newNode = new RingNode(d);
and add that.
Upvotes: 0
Reputation: 14941
You need to create objects that last beyond the scope of your function... so you need to use the new
operator.
RingNode* Ring::Insert(const int& d){
RingNode* refNode = new RingNode(d); // this line creates a ring node not bound to the scope of the function.
if (tail==null){
tail = refNode;
newNode.next = refNode;
return refNode;
}
newNode.next = (*GetTail()).next;
(*GetTail()).next = refNode;
tail = refNode;
return refNode;
}
Upvotes: 1
Reputation: 1690
You're re-using the same local variable on the stack every time.
RingNode newNode(d); //This line overwrites previous RingNode objects
is a local variable - it lives on the stack. So it's only valid during the lifetime of your insert method. However, since you're calling insert several times in a row from the same calling function, your different "newNode"'s wind up at the same place on the stack.
What you probably want to do is
RingNode *refNode = new RingNode(d);
This will dynamically allocate your RingNode on the heap.
However, now you have to worry about using delete to clean up all the nodes when your Ring is destroyed.
Upvotes: 2
Reputation: 43311
RingNode newNode(d); RingNode* refNode = &newNode;
Replace with:
RingNode* refNode = new RingNode(d);
BTW, replace
RingNode(const int& i=0 )
with:
RingNode(int i=0 )
There is no need to use reference for small type like integer.
Upvotes: 1