Help with Destructor for a 2D Array?

[tibrado]

I have a class which has two 2D arrays (int ** M_Array and int** M_Temp). Everything works fine in except when running the destructor. I am aware that I should delete two arrays in the destructor to prevent memory leaks. My problem is:
1. When I run the destructor to delete the arrays it freezes the program. I think I am missing something. But I don’t know what.

Matrix::~Matrix() {
	std::cout << "Destructor" << std::endl; 

	for (int r = 0; r < row_size; r++)
		delete[] M_Temp[r];
		
	delete[] M_Temp; 
};

2. Is it best to re-write the program without pointers?

[tibrado]

Header

#include <iostream>

class Matrix {
private: 
	double ** M_Array, **M_Temp;
	int row_size, col_size; 
	bool evaluate(int, int); // verify size of col and row
	double det_nxn(int, double**); // Determinate nxn calculation functions 
	double inv_nxn(int, double, double**); // Inverse nxn calculation functions 
	bool m_temp_array_size(int, int); 
public:

	Matrix();						// null constructor 
	Matrix(int, int);				// create matrix with size constructor
	Matrix(int, int, double*);		// create matrix with size and values array constructor 
	~Matrix();						// destructor 

	// Operator Overload 
	Matrix operator+ (Matrix);
	Matrix operator- (Matrix);
	Matrix operator* (Matrix);

	// Return Matrix
	Matrix Transposition(); // return matrix transposition 
	Matrix Identity();		// return matrix Identity 
	Matrix Inverse();		// return matrix Invere
	Matrix Pow(int);        // return matrix raised to power of int 
	double Determinant();	// return matrix determinant 

	double get_val(int, int);	// get value at location 

	int get_row_size();		// return matrices row size
	int get_col_size();		// return matrices col size

	bool set_val(int, int, double);  // set value for matrices
	bool set_matrix_val(double *);	 // input a double array into M_Array 
	bool change_m_size(const int, const int); // change Matrix size 

};

// * overload to allow single scaler multiplication. 
Matrix operator* (Matrix& M, const double d);
Matrix operator* (const double& d, Matrix& M);

[tibrado]

#include "MMatrix.h"
Matrix operator* (Matrix& M, const double d) {

	Matrix temp(M.get_row_size(), M.get_col_size()); // create matrix

	for (int r = 0; r < M.get_row_size(); r++) {
		for (int c = 0; c < M.get_col_size(); c++) {
			temp.set_val(r, c, (M.get_val(r, c) * d));
		}
	}
	return temp;
};
Matrix operator* (const double& d, Matrix& M) {
	return M*d;
};

Matrix::Matrix(int row, int col, double* vals) {
	int i = 0; 
	row_size = row;
	col_size = col;

	M_Array = new double*[row];

	// create and add value matrix
	for (int r = 0; r < row; r++) {
		M_Array[r] = new double[col];
		for (int c = 0; c < col; c++) {
			M_Array[r][c] = vals[i];
			i++;
		}
	}

	m_temp_array_size(row, col);
};

Matrix::Matrix(int row, int col) {
	row_size = row;
	col_size = col;

	M_Array = new double*[row];
	// create a NULL matrix
	for (int r = 0; r < row; r++) {
		M_Array[r] = new double[col];
		for (int c = 0; c < col; c++) {
			M_Array[r][c] = 0;
		}
	}

	m_temp_array_size(row, col);
};

Matrix::Matrix() {
	col_size = 1; 
	row_size = 1; 

	M_Array = new double*[row_size];
	M_Array[0] = new double[col_size];
	M_Array[0][0] = 0;

	m_temp_array_size(1, 1);
}
Matrix::~Matrix() {
	std::cout << "Destructor" << std::endl; 

	for (int r = 0; r < row_size; r++)
		delete[] M_Temp[r];
		
	delete[] M_Temp; 

	//delete[] M_Temp;
	//delete[] M_Array; 
};
Matrix Matrix::operator+ (Matrix M) {
	if (M.get_row_size() == row_size && M.get_col_size() == col_size) {
		Matrix temp(row_size, col_size);
		for (int r = 0; r < row_size; r++) {
			for (int c = 0; c < col_size; c++) {
				temp.set_val(r, c, get_val(r, c) + M.get_val(r, c));
			}
		}
		return temp;
	}
	else {
		Matrix temp;
		return temp;
	}
};
Matrix Matrix::operator- (Matrix M) {
	Matrix temp(row_size, col_size);

	if (M.get_row_size() == row_size && M.get_col_size() == col_size) {
		for (int r = 0; r < row_size; r++) {
			for (int c = 0; c < col_size; c++) {
				temp.set_val(r, c, get_val(r, c) - M.get_val(r, c));
			}
		}
		return temp;
	}
	else {
		Matrix temp(1, 1);
		return temp;
	}
};

// * operator overload = multiply MM objects
Matrix Matrix::operator* (Matrix M) {
	double temp_val = 0;

	if (col_size == M.get_row_size()) {
		Matrix temp(row_size, M.get_col_size()); // create matrix
		for (int p = 0; p < M.get_col_size(); p++) {
			for (int n = 0; n < row_size; n++) {
				temp_val = 0;
				for (int m = 0; m < M.get_row_size(); m++) {
					temp_val += M.get_val(m, p) * get_val(n, m);
				}
				temp.set_val(n, p, temp_val);
			}
		}
		return temp;
	}
	else if (row_size == M.get_col_size()) {

		Matrix temp(M.get_row_size(), col_size); // create matrix

		for (int p = 0; p < col_size; p++) {
			for (int n = 0; n < M.get_row_size(); n++) {
				temp_val = 0;
				for (int m = 0; m < row_size; m++) {
					temp_val += get_val(m, p) * M.get_val(n, m);
				}
				temp.set_val(n, p, temp_val);
			}
		}
		return temp;
	}
	else {
		Matrix temp(1, 1);
		return temp;
	}

};

// Return Matrix Transposition 
Matrix Matrix::Transposition() {
	Matrix temp(col_size, row_size);

	for (int r = 0; r < row_size; r++)
		for (int c = 0; c < col_size; c++)
			temp.set_val(c, r, get_val(r, c));

	return temp;
}

// Return matrix Identity 
Matrix Matrix::Identity() {
	if (col_size == row_size && row_size > 1) {
		Matrix temp(row_size, col_size);
		for (int r = 0; r < row_size; r++)
			for (int c = 0; c < col_size; c++) {
				if (r == c)
					temp.set_val(r, c, 1);
				else
					temp.set_val(r, c, 0);
			}
		return temp;
	}
	else {
		return *this;
	}
}

// Return matrix Identity 
Matrix Matrix::Inverse(){
	double det = this->Determinant();

	if (det != 0) {
		Matrix Temp(row_size, col_size);
		if (row_size == col_size) {
			M_Temp = new double*[row_size];
			// create a NULL matrix
			for (int r = 0; r < row_size; r++) {
				M_Temp[r] = new double[col_size];
				for (int c = 0; c < col_size; c++) {
					M_Temp[r][c] = 0;
				}
			}

			if (row_size == 2) {
				Temp.M_Array[0][0] = (1 / det)*this->M_Array[1][1];
				Temp.M_Array[0][1] = (1 / det)*-this->M_Array[0][1];
				Temp.M_Array[1][0] = (1 / det)*-this->M_Array[1][0];
				Temp.M_Array[1][1] = (1 / det)*this->M_Array[0][0];
			}
			else {
				inv_nxn(row_size, det, M_Array);
				Temp.M_Array = this->M_Temp;
				Temp = Temp.Transposition();
			}

			for (int r = 0; r < row_size; r++)
				for (int c = 0; c < col_size; c++)
					Temp.M_Array[r][c] = (1 / det)*(Temp.M_Array[r][c]);
		}
		return Temp;
	}

	return Matrix();
}

// Return Matrix Determinate 
double Matrix::Determinant() {
	if (row_size == col_size && row_size >= 2)
		return det_nxn(row_size, M_Array);
	else
		return 0;
};

// Raise Matrix to Power of input int 
Matrix Matrix::Pow(int power) {
	Matrix temp;

	if (col_size == row_size) {
		temp = *this;
		for (int i = 1; i < power; i++)
			temp = temp * temp;
		return temp;
	}
	else {
		return temp;
	}

}

// Change matrix row and col 
bool Matrix::change_m_size(const int row, const int col) {
	if (row > 0 && col > 0) {
		row_size = row;
		col_size = col;
		return true;
	}
	else {
		return false;
	}

};

// evaluate usr input
bool Matrix::evaluate(int r, int c) {
	if ((r < row_size && r >= 0) && (c < col_size && c >= 0))
		return true;
	else
		return false;
};

// inpute a double array into M_Array 
bool Matrix::set_matrix_val(double *inval) {
	int  i = 0; 
	for (int r = 0; r < row_size; r++)
		for (int c = 0; c < col_size; c++) {
			M_Array[r][c] = inval[i];
			i++; 
		}
	return true; 
}

// return row size
int Matrix::get_row_size() {
	return row_size;
};

// return col size
int Matrix::get_col_size() {
	return col_size;
};

// Return value of matrix at location
double Matrix::get_val(int row, int col) {
	return M_Array[row][col];
};

// Return Determinate n x n matrix
double Matrix::det_nxn(int new_row, double** sent_matrix) {
	int nr, nc, sign = 1;
	double val = 0;
	bool pos = true; 

	if (new_row == 2) {
		return sent_matrix[0][0] * sent_matrix[1][1] - sent_matrix[1][0] * sent_matrix[0][1];
	}
	else {
		// make new array matrix 
		double **temp;
		temp = new double *[new_row];
		for (int r = 0; r < new_row; r++) {
			temp[r] = new double[new_row];
			for (int c = 0; c < new_row; c++) {
				temp[r][c] = 0;
			}
		}

		// find determinate 
		for (int i = 0; i < new_row; i++) {
			nr = 0;
			for (int r = 1; r < new_row; r++) {
				nc = 0;
				for (int c = 0; c < new_row; c++) {
					if (c == i) {
						continue;
					}
					temp[nr][nc] = sent_matrix[r][c];
					nc++;
				}
				nr++;
			}

			val += sign * sent_matrix[0][i] * det_nxn(new_row - 1, temp);
			sign = -sign;

		}
	}

	return val;
}

// Return Inverse n x n matrix
double Matrix::inv_nxn(int new_row, double det,  double** sent_matrix) {
	int nr, nc, sign = 1;

		if (new_row == 2)
			return sent_matrix[0][0] * sent_matrix[1][1] - sent_matrix[1][0] * sent_matrix[0][1];
		else {
			// make new array matrix 
			double **temp;

			temp = new double *[new_row];
			for (int r = 0; r < new_row; r++) {
				temp[r] = new double[new_row];
				for (int c = 0; c < new_row; c++) {
					temp[r][c] = 0;
				}
			}

			// find Cofactors / Minors
			for (int j = 0; j < row_size; j++) {
				for (int i = 0; i < row_size; i++) {
					nr = 0;
					for (int r = 0; r < new_row; r++) {
						if (j == r) {
							continue;
						}
						nc = 0;
						for (int c = 0; c < new_row; c++) {
							if (c == i) {
								continue;
							}
							temp[nr][nc] = sent_matrix[r][c];
							nc++;
						}
						nr++;
					}
					M_Temp[j][i] = sign * inv_nxn(new_row - 1, det, temp);
					sign = -sign;
				}
			}
		}
	return 0; 
}

bool Matrix::set_val(int row, int col, double new_value) {
	if (evaluate(row, col)) {
		M_Array[row][col] = new_value;
		return true;
	}
	else
		return false;
};

bool Matrix::m_temp_array_size(int row, int col) {

		M_Temp = new double*[row];
		// create a NULL matrix
		for (int r = 0; r < row; r++) {
			M_Temp[r] = new double[col];
			for (int c = 0; c < col; c++) {
				M_Temp[r][c] = 0;
			}
		}

		return true; 
}

[tibrado]

main

#include <iostream>
#include "MMatrix.h"


using namespace std;

//1 2 3
//4 5 6
//7 8 9

int main() {

	double num[9] = {2, 2, 3, 4, 5, 6, 7, 8 ,9 }; 
	
	Matrix A(3, 3, num), B;


	A.Inverse();
	


	system("pause");
	return 0; 
}

[doug4]

I didn't do a complete, thorough inspection of your code, so I might have missed something.

However, it looks like you initialize M_Array in your constructors, but don't initialize M_Temp. Then in your destructor, you delete M_Temp and its "contents". Since you never initialized M_Temp, that pointer is garbage, and its contents, when treated as pointers themselves, also point to garbage.

You need to initialize M_Temp to nullptr and then check for nullptr in the destructor before dereferencing M_Temp.

[keskiverto]

Furthermore, there is Rule of Three: https://en.cppreference.com/w/cpp/language/rule_of_three

That is, you cannot accept the default copy constructor and copy assignment.

[Cubbi]

tibrado wrote:

Is it best to re-write the program without pointers?

yes: you would avoid new/delete and associated problems (modern C++ never uses them explicitly), you'd also not need to write destructor, copy constructor, or assignment operator (you'd be following "rule of zero" mentioned on the page keskiverto linked above, instead of "rule of three" that you must follow now because of the use of pointers).

Your basic choice here is between using an array (not pointer) as the underlying storage, which makes the size of the matrix part of its type, and then you template on it:

template<int R, int C>
class Matrix {
private: 
	double M_Array[R][C];
/* ... */
Matrix<3,3> A(num);
Matrix<3,3> B = A.Inverse();

or using a std::vector or std::valarray as the underlying storage, which makes the size a constructor argument/data member as you had it. Use a 1D vector/valarray as the backing storage, it's much easier to manage (and faster to use):

class Matrix {
private: 
	std::vector<double> M_Array;
	int row_size, col_size; 
/* ... */
        // the constructor
        Matrix(int row, int col) : M_Array(row*col), row_size(row), col_size(col) {}
/* ... */
        // the getter
        double get_val(int r, int c) const {
            return M_Array[r*col_size + c]; 
        }
/* ... */
Matrix A(3, 3, num);
Matrix B = A.Inverse();

[tibrado]

Thanks for the replies guys. I have decide to rewrite the pointers arrays into arrays templates. Cubbi right when he says I would not have to deal with the new and delete issues. Thanks again.