we just received the results of our final exam in C++. One of the questions was to write a simple representation of 2-dimensional matrix of double.
I had some points deduced at the destructor implementation with the note reading invalid implementation and no further information. Could you please tell me where I went wrong (if I did indeed go wrong).
This is the code I wrote:
~Matrix() {
for (int i=0; i<_rows; i++) {
for (int j=0; j<_cols; j++) {
delete _arr[i][j];
}
delete _arr[i];
}
delete [] _arr;
}
The code for allocating the memory is pretty straight forward, I'll paste it to make things clearer, but the problematic part is in the code above.
Matrix(int rows, int cols) :
_arr(new double*[rows]), _rows(rows), _cols(cols)
{
for (int i=0; i<_rows; i++) {
_arr[i] = new double[_cols];
}
}
Did they rightfully take away the points? Was my dtor implementation indeed invalid?
3 Answers
The delete _arr[i][j]delete _arr[i]; is also wrong - since this is an array that you allocated with new [], you need to use delete [] _arr[i];.
The rule of thumb is: a delete for each new; and a delete[] for each new[].
Also the deletes of composites are to be executed in reverse order compared to the news (which you did already).
Thus we get
~Matrix() {
for (int i = 0; i < _rows; i++) {
delete[] _arr[i];
}
delete[] _arr;
}
You have two new then three delete. What's wrong should be obvious.
Besides, you have to use delete [] for both dimensions, since you have an array of double-array and then an array of double (double*[] and double[]), that lead you to :
~Matrix() {
for (int i = 0; i < _rows; i++) {
delete[] _arr[i];
}
delete[] _arr;
}
It's not necessary to destroy each value of the array of double, since they have'nt been created by a new
Moreover, a simplest implementation uses only one allocation :
Matrix(int rows, int cols) : _arr(new double[rows*cols]), _rows(rows), _cols(cols)
{
}
~Matrix() {
delete[] _arr;
}
In this implementation, you should provide a correct accessor, since matrix[i,j] = _arr[i+j*row] or matrix[i,j] = _arr[i*cols+j] (depends on the way you arrange the values in the array).
You should call delete[] for your every new[], they should be pairing.
~Matrix() {
for (int i=0; i<_rows; i++) {
delete[] _arr[i];
}
delete [] _arr;
}
