destructor/design problem?

[Tino]

In the snippet below, if the problem line is present, my program
crashes during the Matrix operator*(const Matrix&) call. Would
someone point out to me why I'm having this problem?

Regards,
Ryan

#include <iostream>
#include <cmath>
#include <vector>
#include <exception>
#include <ctime>

using std::cout;
using std::cerr;
using std::endl;

class Matrix
{
private:
int m, n;
double *buffer;

public:
// Constructors
Matrix(int m_, int n_) : m(m_), n(n_), buffer(new double[m*n]){}
~Matrix()
{
if( buffer != NULL )
{
//delete []buffer; //<<<--- This is the problem line
}
}
Matrix& operator=(const Matrix&);
Matrix operator*(const Matrix&) const;
void fill_random();
};

Matrix& Matrix:perator=(const Matrix& A)
{
n = A.n;
m = A.m;

if( buffer != NULL ){
delete [] buffer;
buffer = NULL;
}

buffer = new double[m*n];
for(int i=0; i<n*m; i++)
buffer = A.buffer;

return *this;
}

Matrix Matrix:perator*(const Matrix& A) const
{
Matrix C(m, A.n);
if(n == A.m)
for( int i=0; i<m; i++ )
for( int j=0; j<A.n; j++ )
for( int k=0; k<n; k++ )
C.buffer[i+j*C.m] += buffer[i+k*m]*buffer[k+j*C.m];
else
throw std::length_error("length_error exception in
Matrix:perator*(Matrix)");
return C;
}

void Matrix::fill_random()
{
long seed;
time(&seed);
srand(seed);
for( int i=0; i<m*n; ++i)
buffer = double(rand()) / RAND_MAX;
}

int main()
{
int n = 3;

Matrix A(n,n);
Matrix B(n,n);
Matrix C(n,n);

try{
A.fill_random();
B.fill_random();
C = A * B;
}
catch( exception& e ){
std::cout << e.what() << std::endl;
}
catch(...){
std::cout << "Unknown exception" << std::endl;
}
return 0;
}

 

[Victor Bazarov]

In the snippet below, if the problem line is present, my program
crashes during the Matrix operator*(const Matrix&) call. Would
someone point out to me why I'm having this problem?

First of all let me note that you've not followed the "Rule of Three"
in your code. More details below.

Regards,
Ryan

#include <iostream>
#include <cmath>
#include <vector>
#include <exception>
#include <ctime>

using std::cout;
using std::cerr;
using std::endl;

class Matrix
{
private:
int m, n;
double *buffer;

public:
// Constructors
Matrix(int m_, int n_) : m(m_), n(n_), buffer(new double[m*n]){}

You simply _must_ define the copy c-tor if you have dynamic memory
used in your class. Read more about "Rule of three".

Matrix(const Matrix& m_) : m(m_.m), n(m_.n),
buffer(new double[m*n])
{
std::copy(m_.buffer, m_.buffer + m*n, buffer);
}

~Matrix()
{
if( buffer != NULL )
{
//delete []buffer; //<<<--- This is the problem line

You're probably deleting it twice. The added copy-c-tor should
take care of that. Restore the delete:
delete[] buffer;

}
}
Matrix& operator=(const Matrix&);
Matrix operator*(const Matrix&) const;
void fill_random();
};

Matrix& Matrix:perator=(const Matrix& A)
{
n = A.n;
m = A.m;

if( buffer != NULL ){
delete [] buffer;
buffer = NULL;
}

buffer = new double[m*n];
for(int i=0; i<n*m; i++)
buffer = A.buffer;

return *this;
}

Matrix Matrix:perator*(const Matrix& A) const
{
Matrix C(m, A.n);
if(n == A.m)
for( int i=0; i<m; i++ )
for( int j=0; j<A.n; j++ )
for( int k=0; k<n; k++ )
C.buffer[i+j*C.m] += buffer[i+k*m]*buffer[k+j*C.m];
else
throw std::length_error("length_error exception in
Matrix:perator*(Matrix)");
return C;
}

void Matrix::fill_random()
{
long seed;
time(&seed);
srand(seed);
for( int i=0; i<m*n; ++i)
buffer = double(rand()) / RAND_MAX;
}

int main()
{
int n = 3;

Matrix A(n,n);
Matrix B(n,n);
Matrix C(n,n);

try{
A.fill_random();
B.fill_random();
C = A * B;
}
catch( exception& e ){
std::cout << e.what() << std::endl;
}
catch(...){
std::cout << "Unknown exception" << std::endl;
}
return 0;
}

HTH

Victor

 

[Ivan Vecerina]

In the snippet below, if the problem line is present, my program
crashes during the Matrix operator*(const Matrix&) call. Would
someone point out to me why I'm having this problem?

I haven't checked everything in detail, but your class
lacks a copy-constructor:
Matrix( Matrix const& orig );
(implementation shall be similar to the assignment op,
except that there are no previous contents to destroy).

~Matrix()
{
if( buffer != NULL )
{
//delete []buffer; //<<<--- This is the problem line
}
}

NB: the test for NULL above is redundant:
delete[] NULL; is required to be a no-op.

Matrix& Matrix:perator=(const Matrix& A)
{
n = A.n;
m = A.m;

if( buffer != NULL ){
delete [] buffer;
buffer = NULL;
}

NB: this assignment operator is unsafe in case of
self-assignment, and also isn't exception-safe.
The best way to solve both problems is to
allocate the new buffer first, copy the
contents, then release the old buffer.

hth,