0

i'm having problems creating a destructor for a template class that has a private 2d dynamic array. for some reason the destructor destroys the matrix as soon as i am done entering information into the matrix. not sure what went wrong since it compiles fine but gives an error when i enter the info for the first 2 matrices and the program tries to multiply them. the code works if i get rid of the destructor.

template <class T>
class matrix
{
//sudo
friend matrix operator + , *,-,(bunch of friends used to overload)
//end sudo 
public:
                matrix(): rows(0), cols(0){}
                int Arows(){return rows;}
                int Acols(){return cols;}
            class Proxy
            {
                matrix& _a;
                int _i;
            public:
                Proxy(matrix& a, int i) : _a(a), _i(i){}
                int& operator[](int j) {return _a.Array[_i][j];};
            };
                Proxy operator[](int i) {return Proxy(*this,i);}
                ~matrix();
                private:
                T ** Array;
                int rows;
                int cols;
                };

                template<class T>
                matrix<T>::~matrix()
            {
                for (int i=0;i<rows;i++)
                    delete [] Array[i];
                delete [] Array;
            }
blu
  • 353
  • 1
  • 5
  • 17

2 Answers2

0

It is hard to tell without seeing the exact code and calling code, but it might be because you are missing copy constructor and assignment operator.

matrix m1;    
{
 matrix m2 = ...
 m1 = m2; // copy ctor, now they share same Array pointer
} // m2 destructor free the Array

// m1 is left with dangling Array pointer

from the code sample it doesn't seem like real code. Where is Array initialized, what is matrixArray?

Zdeslav Vojkovic
  • 14,391
  • 32
  • 45
-1

I think the comment provided by R. Martinho Fernandes answers it... I wanted to point something else out and I'm doing it as an answer purely so I can format code.

People get obsessed with doing lots of memory allocations just to make a 2D array... But it's slow! If you are doing a lot with these matrices, you will be better off doing a single memory allocation. It goes a bit like this:

Array = (T**)malloc( rows*sizeof(T*) + rows*cols*sizeof(T) );
T* rowData = (T*)(Array + rows);
for( int r = 0; r < rows; r++ ) {
    Array[r] = rowData;
    rowData += cols;
}

And when you want to free this memory:

free((void*)Array);

Naturally, I assume that you're using basic types... In fact you might be better off using typename T instead of class T in the template.

Oh yeah, this makes it really easy to copy data from one matrix (of the same size) to another. You just allocate as above and then do a single memcpy on the data section.

paddy
  • 60,864
  • 6
  • 61
  • 103
  • Sadly, this completely ignores the alignment of `T`. And `typename T` is *exactly the same as* `class T` in a template parameter. – R. Martinho Fernandes Sep 12 '12 at 02:28
  • If you're concerned about alignment, use two `new` calls instead of one `malloc` and/or introduce padding. In practice, the method described has always worked for me. Disappointed in the downvote, but nevermind. As I said this was a comment (with code), not an answer. – paddy Sep 12 '12 at 02:34
  • i'll keep this in mind for future use but it may be to late for now. – blu Sep 12 '12 at 02:40