1

I am new to C++ and working for an assignment, using C++98. I am trying to loop through an array of objects using pointer to array.

It is printing first point correctly, and after that some value.

Here are the code snippets:

struct Point { int x, y; };
int randomNumber(int low, int high ) {
   int randomNumber = low + (rand())/ (RAND_MAX/(high-low));
   cout << setprecision(2);
   return randomNumber;
}

Point *generateCoordinates(int nodesPerBlock) {
    Point cities[nodesPerBlock];
    for (int i = 0; i < nodesPerBlock; i++) {
        cities[i].x = randomNumber(1, 50);
        cities[i].y = randomNumber(1, 50);
    }
    return cities;
}
int main() {
  int nodesPerBlock = 5;
  Point *points = generateCoordinates(nodesPerBlock);
  for (int n = 0; n < (nodesPerBlock-2); n++) {
    cout << "n: x=" << points[n].x << ", y=" << points[n].y << endl;
    cout << "n+1: x=" << points[n+1].x << ", y=" << points[n+1].y << endl;
  }
}

this prints:

n: x=1, y=4
n+1: x=18, y=11
n: x=2049417976, y=32767
n+1: x=2049417976, y=32767
n: x=2049417976, y=32767
n+1: x=2049417984, y=167804927

while the actual Points printed were:

Point : x=1, y=4.
Point : x=18, y=11.
Point : x=13, y=6.
Point : x=2, y=16.
Point : x=16, y=22.

referred this questions and this, but no success so far.

1

1 Answer 1

1

cities[nodesPerBlock] is a local variable in generateCoordinates function and it goes out of scope when the function exits.
You are returning an address to it and are accessing that address in main. This is undefined behavior to do so.

You have to allocate cities on the the heap using new (since you are using C++98) and then return that address to main. Then you will be able to reliably access that address.

After your processing, do not forget to delete the memory you have allocated at the end of main.

You can avoid memory allocation and deletion by changing your function to take an extra parameter which you can pass from main. Then cities can be an array of Points on the stack.

void generateCoordinates(Point cities[], int nodesPerBlock);
Sign up to request clarification or add additional context in comments.

2 Comments

Why not simply void generateCoordinates(Point cities[], int nodesPerBlock)? Dynamic allocation is really not required here. And I'm pretty sure C++98 still has std::vector, which should be the default advice given these days to anyone requiring dynamic contiguous memory on the heap.
I do not know what constraints the OP is under, whether he is permitted to use vector or not. But it is better to have the function signature you suggest. Then there is no need for dynamic memory allocation. I will update the answer to that effect.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.