0

I have created a two dimensional array on the heap. And I want to fill the array with numbers. I have tried two ways of doing this. The first attempt sort of works, but the problem is that the program will crash when I try to delete it. The second attempt that I did made it possible to delete the array after I have used it, but the array is not filled the correct way.

So here is how I declared the two dimensional array:

int** matrix = new int*[rowSize];
for (int i = 0; i < rowSize; i++) {
    matrix[i] = new int[ColSize];
}

And then I fill it like this:

int matrixI0[]{1, 0, 0, 0, 1, 0, 1, 0, 0, 1, 1, 1};
int matrixI1[]{1, 0, 0, 0, 1, 1, 1, 0, 1, 1, 0, 1};
int matrixI2[]{1, 0, 0, 1, 1, 1, 1, 0, 1, 1, 0, 1};
int matrixI3[]{1, 1, 0, 1, 1, 0, 1, 0, 1, 0, 0, 1};
int matrixI4[]{1, 1, 1, 1, 1, 0, 1, 0, 1, 0, 0, 1};
int matrixI5[]{1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 0, 1};
int matrixI6[]{0, 0, 1, 0, 0, 0, 1, 1, 1, 0 ,0, 1};


matrix[0] = matrixI0;
matrix[1] = matrixI1;
matrix[2] = matrixI2;
matrix[3] = matrixI3;
matrix[4] = matrixI4;
matrix[5] = matrixI5;
matrix[6] = matrixI6;

When I am filling it this way the array is behaving the way I want and I am getting the expected result in a later method that uses this array.

However when I try to delete it:

    for (int i = 0; i < rowSize; ++i) {
    delete[] matrix[i];
}
delete[] matrix;

I get an thrown exception with the following error message "Test.exe has triggered a breakpoint." And of course if I run it in release the program will crash.

My second attempt of filling the array looked like this:

int matrixI0[]{1, 0, 0, 0, 1, 0, 1, 0, 0, 1, 1, 1};
int matrixI1[]{1, 0, 0, 0, 1, 1, 1, 0, 1, 1, 0, 1};
int matrixI2[]{1, 0, 0, 1, 1, 1, 1, 0, 1, 1, 0, 1};
int matrixI3[]{1, 1, 0, 1, 1, 0, 1, 0, 1, 0, 0, 1};
int matrixI4[]{1, 1, 1, 1, 1, 0, 1, 0, 1, 0, 0, 1};
int matrixI5[]{1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 0, 1};
int matrixI6[]{0, 0, 1, 0, 0, 0, 1, 1, 1, 0 ,0, 1};


matrix[0][0] = *matrixI0;
matrix[1][0] = *matrixI1;
matrix[2][0] = *matrixI2;
matrix[3][0] = *matrixI3;
matrix[4][0] = *matrixI4;
matrix[5][0] = *matrixI5;
matrix[6][0] = *matrixI6;

And now there is no problem when I am deleting the array, but now the array is not behaving the way I want it to, and when I test the method that uses the array I get the same result as if the array would just have been filled with zeros.

I know that I am not doing it right in the second attempt but I just did this to see what had to be done so the array could be succesfully deleted.

So to sum this up my question is how should I fill the two dimensional array in a way that it is correctly filled, but it will also succesfully be deleted?

Thanks!

Jacce
  • 172
  • 1
  • 2
  • 11
  • It is because the equal operator does not do all thé work for you. In the first case you copy the pointer in the second case you only copy thé first value of the array. – PilouPili Oct 25 '18 at 16:10
  • You may want to know about [std::fill](https://en.cppreference.com/w/cpp/algorithm/fill). – Jesper Juhl Oct 25 '18 at 16:11
  • Your method of creating your 2D array is one of the worst ways of creating one. You are calling the allocator / deallocator `rowSize + 1` times thus fragmenting the heap, slowing your creation / deallocation down, and your matrix has no chance of taking advantage of cache locality due to the data being non-contiguous. [See this answer](https://stackoverflow.com/questions/21943621/how-to-create-a-contiguous-2d-array-in-c/21944048#21944048) for an alternative in creating a 2d array from a `T**`. – PaulMcKenzie Oct 25 '18 at 17:16
  • @PaulMcKenzie Thanks, your answer solved my problem. However I found the code in your answer to be a little complex and difficult for me. But I guess I will learn eventually. :) – Jacce Oct 25 '18 at 17:47
  • @Jacob -- It should be easy conceptually as to the way it works. Instead of allocating each row, allocate the whole matrix in one shot. Then you allocate your row pointers and then strategically point them inside of this matrix, thus simulating a 2D array. – PaulMcKenzie Oct 25 '18 at 17:50

2 Answers2

2

Fast answer: don't use raw arrays, it's C++, just use std::vector<int>/std::array<int> which properly overloads operator= and they will do the work for you, eg:

std::vector<std::vector<int>> matrix(7);
matrix[0] = { 1, 3, 3};

Long answer:

You are not filling the arrays on the heap, rather you are replacing their reference in matrix with local arrays which get deleted when exiting the scope and can't be deleted at all in any case.

To explain the issue let's keep it simple, suppose a single array on the heap

int* row = new int[rowSize];

So you have a variable row which stores a memory address to the array allocated on heap:

| row | ------> |x|x|x|x|x|x|x|

Now you do:

int myRow[] = { 1, 2, 3};
row = myRow;

and what happens is that myRow is allocated on stack and its address is stored inside row variable which ends up with a situation like:

| row |         |x|x|x|x|x|x|x|
   |
   --> |y|y|y|y|y|y|y|

So you've lost any reference to the array on heap and row points to an address on stack. You can't delete row anymore and data pointed by it will become garbage when exiting the scope. You didn't copy anything in the end.

To properly copy data you could use std::copy, eg:

int myRow[] = { 1, 2, 3};
std::copy(myRow, myRow + rowSize, row);
Jack
  • 131,802
  • 30
  • 241
  • 343
2

< Mandatory "You should just use a 2D std::vector instead!!" rant (no but really, all of this would be easier if you just used a vector) >

Let's look at what you're doing for one of the elements:

matrix[2] = new int[ColSize];

matrix[2] (which is an int*) is given the address of a new array you've allocated on the heap.

matrix[2] = matrixI2;

Throw out the old value of matrix[2] (the pointer to the array on the heap), causing the old dynamically allocated array to be lost, and point matrix[2] to the local array, matrixI2.

If matrixI2 goes out of scope, our pointer is now invalid. Also, there's no need to delete matrix[2], since it's not pointing to dynamically allocated memory.

Instead what you want is probably:

matrix[2] = new int[12]{1, 0, 0, 1, 1, 1, 1, 0, 1, 1, 0, 1};

for each index instead of your loop.

scohe001
  • 15,110
  • 2
  • 31
  • 51