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.

Nitin1706
  • 621
  • 1
  • 11
  • 21
  • Please see https://stackoverflow.com/questions/7769998/how-to-return-local-array-in-c but take the accepted answer with a grain of salt. – paddy Nov 15 '18 at 05:17

1 Answers1

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);
P.W
  • 26,289
  • 6
  • 39
  • 76
  • 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. – paddy Nov 15 '18 at 05:21
  • 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. – P.W Nov 15 '18 at 05:27