Why can't I have pointers to objects, which have the same type of pointer as a member variables?

Question

It sounds a bit weird the way I phrased it, but that's simply because I have no idea how I can phrase it. I'm trying to implement A*, and I did it before but I had one problem with tracing my path back, so I decided to run a small test. The problem went something like this:

I have a class that looks a bit like this:

class Number {
public:
    int xPos;
    int yPos;

    Number *prevNum;
    Number(int x, int y) {
        xPos = x;
        yPos = y;
    }
};

And in the main function, I do this

int main() {
    Number n(2, 2);
    Number *current = &n;
    vector<Number> nums;
    nums.push_back(*current);
    for (unsigned i = 0; i < 15; i++) {
        Number n(current->xPos + 1, current->yPos);
        n.prevNum = current;
        nums.push_back(n);
        current = &n;
        cout << current->xPos + 1 << " ";
    }
    for (unsigned i = 0; i < nums.size(); i++) {
        if (nums.at(i).prevNum) {
            cout << nums.at(i).prevNum->xPos << " ";
        }
    }
    return 0;
}

For some reason, it returns this:

4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 555437610 2 17 17 17 17 17 17 17 17 17 17 17 17 17 17

The 555437610 is different every time, so I assume I might have an error with pointers. Can I not nest the member function *prevNum infinitely? I'm not completely sure how to describe it.

Answer 1

The scope of n is the for loop only. So current = &n; sets current to point to an object which will soon go out of scope.

nums.push_back(n); copies n into the vector, and it is &nums.back() you should assign to current.

In fact, your program can be simplified as follows:

struct Number { // Now a proper aggregate
    int xPos;
    int yPos;

    Number *prevNum;
};

vector<Number> nums {{1, 2, nullptr}};
nums.reserve(16);
for (unsigned i = 0; i < 15; ++i) {
    Number& back = nums.back();
    nums.emplace_back({back.xPos + 1, back.yPos + 1, &back});
    cout << nums.back().xPos + 1 << " ";
}

Answer 2

Your problem is with

current = &n;

Here you are taking the address of a loop local variable. At the end of the iteration that variable is destroyed which means you have a pointer to an object that no longer exists. Dereferencing that pointer is undefined behavior.

If you want to keep the behavior defined then what you should do is store a pointer to the object in the vector. That pointer may/will get invalidated after the call to push_back but since you use it before that it will be okay. You should be okay with

current = &nums.back();

You are also going to have an issue storing the previous pointer. If you capture the element from the vector, if the vector reallocates space then you will be left with a dangling pointer. I think you are going to need some sort of shared_ptr setup in order to get this to work.

Answer 3

You have:

for (unsigned i = 0; i < 15; i++) {
    Number n(current->xPos + 1, current->yPos);
    n.prevNum = current;
    nums.push_back(n);
    current = &n;
    cout << current->xPos + 1 << " ";
}

There, n is a local variable that gets destroyed when the loop ends. You are storing a pointer to the local variable and are using it later.

Your program has undefined behavior.

---

It's not clear to me why you need the prevNum member variable. You can get rid of it altogether.

#include <iostream>
#include <vector>

using namespace std;

class Number {
public:
    int xPos;
    int yPos;

    Number(int x, int y) {
        xPos = x;
        yPos = y;
    }
};

int main() {
   Number n(2, 2);
   vector<Number> nums;
   nums.push_back(n);
   for (unsigned i = 0; i < 15; i++) {
      Number n(2+i+1, 2);
      nums.push_back(n);
      cout << 2 + i+1 << " ";
   }
   cout << endl;
   for (unsigned i = 0; i < nums.size(); i++) {
      cout << nums.at(i).xPos << " ";
   }
   cout << endl;
   return 0;
}