SOLVED: C++ Delete huge double arrays

[fromjavatocpp]

Hi at all here!

I'm new to C++ and as I have coded a lot in Java now I got problems with the memory and although I have been browsing through several pages now, I still didn't find out how to solve my problem the best.

What I'm doing is programming a little maltab clone for my numerical analysis course.
Thus I have to be able to operate on giant matrices.
But somehow I am not able to free memory to the system and my Destructor calls generate memory trouble.

This is my matrix class:

#ifndef INC_MATRIX_H
#define INC_MATRIX_H

#include <iostream>
class Matrix 
{

public:
		int row, col;
		double **array;

		Matrix(int r, int c);
		~Matrix();

		void operator=(Matrix B);
		Matrix transpose();
};
#endif

The implementation is as follows:

#include "str.h"
using namespace std;

Matrix::Matrix(int r, int c) {
		this->row = r; this->col = c;			
		array = new double*[r];
		for(int i = 0; i < r; i++) 
			array[i] = new double[c];
	}
Matrix::~Matrix() {
		this->row = 0;
		this->col = 0;
		//delete this->array;// =new double*[0];; // This does not work somehow!
	}
void Matrix::operator=(Matrix B) {
		// includes some warnings concerning dimensions
		for(int i = 0; i < this->row; i++) 	
			for(int j = 0; j < this->col; j++) 	
				this->array[i][j] = B.array[i][j];
	return;
	}
Matrix Matrix::transpose() {
		Matrix C = *(new Matrix(this->col,this->row));
		for(int i = 0; i < this->row; i++) 	
			for(int j = 0; j < this->col; j++) 	
			 	C.array[j][i] = this->array[i][j];	
		return C;
	}

You can test it using this little programm:

#include <iostream>

#include "str.h"

using namespace std;

int main() {
int n = 20;
Matrix B(n,n);
for(int i = 0; i < n; i++) B.array[i][i] = i;
	while(1) {
		cout << B.array[0][0];
		B=B.transpose();
	}
}

You will see, that the memory usage grows and grows, although I thought it is just one matrix.
Have you got any ideas or links how I can optimise my code?

Kind Regards.
Roland

PS: May someone move this post to the c++ section, please. I posted it wrong. Thank you!

[Athar]

You have to free the memory the same way you allocated it, i.e. one call to delete[] for every call to new[].
So your destructor should look like this:

Matrix::~Matrix()
{
  for (int i = 0; i < row; i++)delete[] array[i];
  delete[] array;
  row = 0;
  col = 0;
}

this-> inside member functions is optional, by the way (unless you have a function parameter with the same name).

[fromjavatocpp]

Ah! Ok. This works fine in my test case, but now I tested this with matrix matrix multiplication and now I get an error which displays me a "memory map".

If you would add the following to the
Header file:

Matrix operator*(Matrix B);

str.cpp file:

Matrix Matrix::operator*(Matrix B) 
	{
		if( this->col != B.row) { 
			cout << "mult: Error, Matrix dimensions do not match: ";
			cout << "Op1 is (" << this->row <<", "<< this->col << "), " << "Op2 is (" << B.row << ", " << B.col << ")" << endl; 
		 return *(new Matrix(0,0)); 
		}
		// initialize the new matrix
		Matrix C = *(new Matrix(this->row, B.col));
		// multiplicate
		for(int i= 0; i < this->row; i++) 	
			for(int j = 0; j < B.col; j ++) 	
				for(int k = 0; k < this->col; k ++) 
					C.array[i][j] += this->array[i][k] * B.array[k][j]; 
		return C;
	}

testcase:

B=B*B;

I've checked, that it is not the problem that the matrix is multiplied with itself or within the loop..

[Athar]

Did you take into account that unlike in Java, "Matrix B" is not a reference? When passing objects this way to a function, a copy of the object is created, which requires a copy-constructor to be defined. If you want to pass the matrix by reference, this is the way to go:

Matrix operator*(const Matrix& B);

In the implementation of operator* you should not create matrixes with new, as this will result in a memory leak. Instead, create them this way:

return Matrix(0,0);
//and:
Matrix C(this->row, B.col);

When you return the matrixes, it's possible that they need to be copied, which again requires a valid copy constructor.

[C++ Forumbot]

Dieser Thread wurde von Moderator/in rüdiger aus dem Forum Rund um die Programmierung in das Forum C++ verschoben.

Im Zweifelsfall bitte auch folgende Hinweise beachten:
C/C++ Forum :: FAQ - Sonstiges :: Wohin mit meiner Frage?

Dieses Posting wurde automatisch erzeugt.

[fromjavatocpp]

Yeah, I almost understand what's happening. Your advice works fine with operator=!!

But I've still've got a memory leak when it comes to multiplication.

May you explain to me, how operator= and the copy constructor work together?

My Copy-Constructor is now

Matrix::Matrix(const Matrix& B) 
		: row(B.row), col(B.col), array(new double*) 
	{
		for(int i = 0; i < B.row; i++) {
			array[i] = new double[B.col];    // #include <algorithm> for std::copy
			for(int j = 0; j < B.col; j++)
				array[i][j] = B.array[i][j];
		}
	}

and I changed multiplication like this:

Matrix Matrix::operator*(const Matrix& B) 
	{
		if( this->col != B.row) { 
			cout << "mult: Error, Matrix dimensions do not match: ";
			cout << "Op1 is (" << this->row <<", "<< this->col << "), " << "Op2 is (" << B.row << ", " << B.col << ")" << endl; 
		 return Matrix(0,0); 
		}
		// initialize the new matrix
		Matrix C(this->row, B.col);
		// multiplicate
		for(int i= 0; i < this->row; i++) 	
			for(int j = 0; j < B.col; j ++) 	
				for(int k = 0; k < this->col; k ++) 
					C.array[i][j] += this->array[i][k] * B.array[k][j]; 
		return C;
	}

If I test it with

while(1) C*B

I still am leaking memory. If you are wondering, why I do not have a Destructor:
I get the memory map error again!

Btw: Do you know why I don't get these fancy code boxes?

Regards,
Roland :xmas1:

[Nexus]

fromjavatocpp schrieb:

I still am leaking memory. If you are wondering, why I do not have a Destructor:
I get the memory map error again!

If you haven't got a destructor which deletes the allocated memory, that's simple. Every multiplication creates a new matrix, therefore calls the constructor. In the constructor, you allocate memory by new that is never freed.

fromjavatocpp schrieb:

May you explain to me, how operator= and the copy constructor work together?

Have you ever heard of Copy&Swap? That's an idiom to implement the assignment operator safely (regarding exceptions), plus to avoid code duplication.

First, provide a swap() method that exchanges the contents of two matrices:

void Matrix::swap(Matrix& other)
{
    std::swap(col, other.col);
    std::swap(col, other.col);
    std::swap(array, other.array);
}

Then, you can implement the assignment operator using your copy constructor and swap() :

Matrix& Matrix::operator= (const Matrix& origin)
{
    Matrix temp(origin); // create a temporary copy of origin
    swap(temp); // exchange contents of *this and temp, so *this equals origin
    return *this;
} // note: temp gets out of scope here, so the old content is destroyed.

You can find more informations about Copy&Swap in the english wikibooks.

fromjavatocpp schrieb:

Btw: Do you know why I don't get these fancy code boxes?

Did you accidentally check the box "BBCode in diesem Beitrag deaktivieren" (deactivate BBCode)?

[fromjavatocpp]

Hi!

Thank you for your advice with the copy and swap. Seems to make sense.

But still the problem is there: Maybe someone can try to get the code to run? (This would be nice!)

When I try to install a Destructor like you can see below, I get a Error:

*** glibc detected *** ./a.out: corrupted double-linked list: 0x09d833a0 ***

I have done the swap thing, too, and my =operator is like it should be and my copy constructor works fine now.

But still, what is wrong with my destructor?

My testprog now is as simple as this:

#include <iostream>
using namespace std;
class Matrix {
public:
int row, col;
double **array;
 Matrix(int r, int c) {
	this->row = r; this->col = c;			
	array = new double*[r];
	for(int i = 0; i < r; i++) 
		array[i] = new double[c];
}
 Matrix(const Matrix& B) 
	: row(B.row), col(B.col), array(new double*) {
	for(int i = 0; i < B.row; i++) {
	   array[i] = new double[B.col];    
	     for(int j = 0; j < B.col; j++)
		array[i][j] = B.array[i][j];
        }
// the whole copy thing here does not work properly, because at the end it would be the same refernce and the problem still exists
}
~Matrix() {
	for (int i = 0; i < row; i++) delete[] array[i];
	delete[] array;
	row = 0;
	col = 0; 
	}
void swap(Matrix& B) 	{
         std::swap(row, B.row);
         std::swap(col, B.col);
         std::swap(array, B.array);
}
Matrix& operator=(const Matrix& B) {
    Matrix temp(B); // create a temporary copy of origin
    swap(temp); // exchange contents of *this and temp, so *this equals origin
    return *this;
}
}; // class

int main() {
   int n = 10;
   Matrix C(n,n);
   for(int i = 0; i < n; i++)
	C.array[i][i] = i;	
   Matrix A = Matrix(C);
   cout << C.array[1][1] << " " << A.array[1][1] << endl;
   C.array[1][1] = 100;
   cout << C.array[1][1] << " " << A.array[1][1] << endl;
}

I just noticed that I get no problem when I do not call the Copy Constructor

[uhsuhz]

There is something strange in your code, in the constructor your wrote:

array = new double*[r];

However in the copyconstructor, you wrote:

array(new double*)

You have never specified how many rows should be created... maybe that could produce an error.

[fromjavatocpp]

On again.. Thank you all!
I think I have learned a lot about C++ from you.

Have a nice Sunday!

Best wishes,
Roland

[Gast]

...