destructor/design problem?

Discussion in 'C++' started by Tino, Jun 24, 2003.

  1. Tino

    Tino Guest

    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::eek: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::eek: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::eek: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;
    }
    Tino, Jun 24, 2003
    #1
    1. Advertising

  2. "Tino" <> wrote...
    > 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::eek: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::eek: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::eek: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
    Victor Bazarov, Jun 24, 2003
    #2
    1. Advertising

  3. "Tino" <> wrote in message
    news:...
    > 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::eek: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,
    --
    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
    Ivan Vecerina, Jun 24, 2003
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. frs
    Replies:
    20
    Views:
    735
    Alf P. Steinbach
    Sep 21, 2005
  2. arun
    Replies:
    2
    Views:
    534
    benben
    Jun 13, 2006
  3. Joseph Turian
    Replies:
    5
    Views:
    432
    Grizlyk
    Jan 15, 2007
  4. Jimmy Hartzell
    Replies:
    0
    Views:
    411
    Jimmy Hartzell
    May 19, 2008
  5. Jimmy Hartzell
    Replies:
    2
    Views:
    1,162
    Jimmy Hartzell
    May 20, 2008
Loading...

Share This Page