In C++, how to I stop my pointers from being 'overridden' when a function is called again?

Question

Hello, 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.

Answer 1

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.

Answer 2

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;
}

Answer 3

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.