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::operato r=(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[i] = A.buffer[i];

return *this;
}

Matrix Matrix::operato r*(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_err or("length_erro r exception in
Matrix::operato r*(Matrix)");
return C;
}

void Matrix::fill_ra ndom()
{
long seed;
time(&seed);
srand(seed);
for( int i=0; i<m*n; ++i)
buffer[i] = 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]

Re: destructor/design problem?

"Tino" <tino52@yahoo.c om> wrote...[color=blue]
> 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?[/color]

First of all let me note that you've not followed the "Rule of Three"
in your code. More details below.
[color=blue]
>
> 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]){}[/color]

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_.bu ffer, m_.buffer + m*n, buffer);
}
[color=blue]
> ~Matrix()
> {
> if( buffer != NULL )
> {
> //delete []buffer; //<<<--- This is the problem line[/color]

You're probably deleting it twice. The added copy-c-tor should
take care of that. Restore the delete:
delete[] buffer;
[color=blue]
> }
> }
> Matrix& operator=(const Matrix&);
> Matrix operator*(const Matrix&) const;
> void fill_random();
> };
>
> Matrix& Matrix::operato r=(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[i] = A.buffer[i];
>
> return *this;
> }
>
> Matrix Matrix::operato r*(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_err or("length_erro r exception in
> Matrix::operato r*(Matrix)");
> return C;
> }
>
> void Matrix::fill_ra ndom()
> {
> long seed;
> time(&seed);
> srand(seed);
> for( int i=0; i<m*n; ++i)
> buffer[i] = 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;
> }[/color]

HTH

Victor

[Ivan Vecerina]

Re: destructor/design problem?

"Tino" <tino52@yahoo.c om> wrote in message
news:f9d112e6.0 306241252.3f446 16d@posting.goo gle.com...[color=blue]
> 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?[/color]

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).
[color=blue]
> ~Matrix()
> {
> if( buffer != NULL )
> {
> //delete []buffer; //<<<--- This is the problem line
> }
> }[/color]
NB: the test for NULL above is redundant:
delete[] NULL; is required to be a no-op.
[color=blue]
> Matrix& Matrix::operato r=(const Matrix& A)
> {
> n = A.n;
> m = A.m;
>
> if( buffer != NULL ){
> delete [] buffer;
> buffer = NULL;
> }[/color]
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,
--
Ivan Vecerina, Dr. med. <> http://www.post1.com/~ivec
Soft Dev Manger, XiTact <> http://www.xitact.com
Brainbench MVP for C++ <> http://www.brainbench.com