0

In SO question [How to allocate a 2D array of pointers in C++] [1], the accepted answer also makes note of the correct procedure of how to de-allocate and delete said array, namely "Be careful to delete the contained pointers, the row arrays, and the column array all separately and in the correct order." So, I've been successfully using this 2D array in a cellular automaton simulation program. I cannot, however, get this array's memory management correct. I do not see an SO answer for how to do this other than the reference above.

I allocate the 2D array as follows:

Object*** matrix_0 = new Object**[rows];
    for (int i = 0; i < rows; i++) {
        matrix_0[i] = new Object*[cols];
    }

My futile attempt(s) (according to Valgrind) to properly de-allocate the above array are as follows:

for (int i = 0; i < rows; i++) {
    for (int j = 0; j < cols; j++) {
        matrix_0[i][j] = NULL;
    }
}
delete [] matrix_0;
matrix_0 = NULL;

Clearly, I'm missing the rows and cols part as reference [1] suggests. Can you show me what I'm missing? Thanks in advance.

[1]: (20 Nov 2009) How to allocate a 2D array of pointers in C++

Community
  • 1
  • 1
RigidBody
  • 656
  • 3
  • 11
  • 26
  • 6
    `std::vector > > vector_of_objects;` – SergeyA Mar 21 '16 at 18:35
  • Do you heed a 2D array of objects, or a 2D array of pointers? – R Sahu Mar 21 '16 at 18:35
  • @ R Sahu: It's a 2D array of pointers to base class objects and is set up this way to leverage polymorphic behavior of the various cellular automaton derived objects. – RigidBody Mar 21 '16 at 18:41
  • @Chris Where is your allocation for the `matrix_0[i]` data? You're not even doing the allocation correctly (or at best, it is incomplete). Also, if the array doesn't change shape after you've set it up, this is far better in terms of efficiency: http://stackoverflow.com/questions/21943621/how-to-create-a-contiguous-2d-array-in-c/21944048#21944048 Just use `Base*` as the type of data. – PaulMcKenzie Mar 21 '16 at 18:43
  • @Paul, yes, you are correct, I left that part out, as the objects have a separate memory de-allocation task themselves and didn't appear germane to this question. – RigidBody Mar 21 '16 at 18:55
  • I have some leading questions... 1) Is `Object` polymorphic? 2) If not, how large is the `sizeof(Object)`? 3) If 'no' and 'small' then why pointers at all? – Richard Hodges Mar 21 '16 at 20:00

2 Answers2

9

You have a tonne of deleting to do in this:

for (int i = 0; i < rows; i++) {
    for (int j = 0; j < cols; j++) {
        delete matrix_0[i][j]; // delete stored pointer
    }
    delete[] matrix_0[i]; // delete sub array
}
delete [] matrix_0; //delete outer array
matrix_0 = NULL;

There is no need to NULL anything except matrix_0 because they are gone after delete.

This is horrible and unnecessary. Use a std::vector and seriously reconsider the pointer to the contained object.

std::vector<std::vector<Object*>> matrix_0(rows, std::vector<Object*>(cols));

Gets what you want and reduces the delete work to

for (int i = 0; i < rows; i++) {
    for (int j = 0; j < cols; j++) {
        delete matrix_0[i][j]; // delete stored pointer
    }
}

But SergeyA's suggestion of storing unique_ptr, std::vector<std::vector<std::unique_ptr<Object>>> matrix_0; reduces the deletions required to 0.

Since speed is one of OP's goals, there is one more improvement:

std::vector<std::unique_ptr<Object>> matrix_0(rows * cols);

Access is

matrix_0[row * cols + col];

This trades a bit of visible math for the invisible math and pointer dereferences currently going on behind the scenes. The important part is the vector is now stored as a nice contiguous block of memory increasing spacial locality and reducing the number of cache misses. It can't help with the misses that will result from the pointers to Objects being scattered throughout memory, but you can't always win.

A note on vector vs array. Once a vector has been built, and in this case it's all done in one shot here:

std::vector<std::unique_ptr<Object>> matrix_0(rows * cols);

all a vector is is a pointer to an and a couple other pointers to mark end and the the location of the last location used. Access to the data array is no different from access to a dynamic array made with new. Using the index operator [] compiles down to data_pointer + index exactly the same as using [] on an array. There is no synchronizing or the like as in Java's Vector. It is just plain raw math.

Compared to a dynamic array all a preallocated vector costs you is two pointers worth of memory and in return you get as close to no memory management woes as you are likely to ever see.

user4581301
  • 33,082
  • 7
  • 33
  • 54
  • Yes, horrible but made necessary, I'm told, by "speed" concerns. Thank you for your prompt and correct (tested) answer. – RigidBody Mar 21 '16 at 18:49
  • 1
    The speed difference between this and `std::vector>` is likely somewhere between "undetectable" and "negligible" if you reserve sufficient space when you construct your vectors. – Miles Budnek Mar 21 '16 at 18:55
0

Before setting the pointers to NULL, you should delete them first. After every pointer in the column are deleted, you can delete[] the row and set it to NULL, as every element is deleted and gone.

Rakete1111
  • 47,013
  • 16
  • 123
  • 162