Deallocating Dynamic 2D Array with Template

I am having an error in my C++ application that I am having trouble debugging. I have looked online, and I appear to b开发者_开发百科e doing all of my allocation/deallocation the correct way. Here is my code:

template <typename T>
class Matrix
{
private:
   int _rows;
   int _cols;
   T** _matrix;
public:
   Matrix(int r, int c);
   ~Matrix();
   T GetValue(int r, int c);
};

template <typename T>
Matrix<T>::Matrix(int r, int c)
{
   _rows = r;
   _cols = c;

   _matrix = new T*[_rows];
   for(int i = 0; i < _rows; i++)
      _matrix[i] = new T[_cols];

   for(int i = 0; i < _rows; i++)
      for(int j = 0; j < _cols; j++)
         _matrix[i][j] = NULL;
}

template <typename T>
Matrix<T>::~Matrix()
{
   for(int i = 0; i < _rows; i++)
      delete [] _matrix[i];
   delete [] _matrix;
}

template <typename T>
T Matrix<T>::GetValue(int r, int c)
{
   if(r < 0 || r >= _rows || c < 0 || c > _cols)
   {
      throw -1;
      return NULL;
   }

   return _matrix[r][c];
}

And my client code...

int main()
{
   Matrix<int> myMatrix(3, 3);
   myMatrix.GetValue(1, 1);
   // myMatrix.~Matrix();  // Don't do this anymore
}

As soon as the variable "myMatrix" goes out of scope, I get this error:

Unhandled exception at 0x103159da (msvcr1000d.dll)...Access violation reading location 0xfeeefee2.

And I am brought to the file "dbgdel.cpp" _ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse));

Please help!

---

EDIT:

Okay, I neglected to give some information. Please see below:

I have an additional method called "T Dot(Matrix)" I've also got two methods called "Columns()" and "Rows()" which are just getters for _cols and _rows. And a method called "SetValue(int r, int c, T value)" which sets _matrix[r][c] = value. I don't think showing the implementation is necessary for these.

template <typename T>
T Matrix<T>::Dot(Matrix<T> m)
{
   if(_cols > 1 || m.Columns() > 1 || _rows != m.Rows())
   {
      throw -1;
      return NULL;
   }

   T value = 0;
   for(int i = 0; i < _rows; i++)
   {
      value += _matrix[i][0] * m.GetValue(i, 0);
   }
   return value; // Whoops, this was here, just forgot to type it
}

And the client...

int main()
{
   Matrix<int> intM1(3, 1);
   Matrix<int> intM2(3, 1);

   intM1.SetValue(0, 0, 1);
   intM1.SetValue(1, 0, 1);
   intM1.SetValue(2, 0, 1);
   intM2.SetValue(0, 0, 1);
   intM2.SetValue(1, 0, 1);
   intM2.SetValue(2, 0, 1);

   std::cout << intM1.Dot(intM2) << endl;
}

This generates the same error as above, but only when the "Dot()" function is called.

---

Don't call the destructor explicitly, you've allocated myMatrix on the stack, so the dtor will automatically be called when the variable goes out of scope, i.e. when main() returns.

---

With your edit, the issue here is still the lack of a copy constructor and assignment operator (see my answer to your last question). The problem is that when you pass the Matrix by value into a function, the copy constructor is invoked to make the copy, but since you haven't defined one C++ will use the default copy constructor, which just makes a shallow copy. Consequently, you'll end up getting a new Matrix that shares a pointer to the same elements as the old Matrix. When this new Matrix goes out of scope, its destructor will fire, cleaning up the array used by the other Matrix. When the original Matrix then goes out of scope and is cleaned up, it will try to delete an already-deleted array, causing the crash.

To fix this, you need to implement a copy constructor and assignment operator that correctly duplicate the resources. There's a rule-of-thumb called the Rule of Three that says that if you have a destructor, you also need a copy constructor and assignment operator to prevent these sorts of bugs. Try implementing these missing functions and see if the problem clears up.

---

Edit:

Here is the error:

T Matrix<T>::Dot(Matrix<T> m);

This function should return T and it's not returning in the end !! Put,

return value;

And it should be solved. Also there are some suggestions: You are passing Matrix<T> m; by value which is error prone. Because both the copied m and original intM2 will point to same _matrix. So when m goes out of scope it will delete[] everything. When intM2 goes out of scope, it will again delete the same memory. Which again a crash.

So you should always specify copy constructor (either as private or with proper copying code). As of now change the definition to,

T Matrix<T>::Dot(Matrix<T> &m); // pass `m` by reference

This will resolve all your errors. (You can choose to pass by const Matrix<T> &m also, for that you may have to change GetValue () const.)