CPP: Matrix Destructor

Question

For the following mymatrix class definition, why don't I need the commented parts of the destructor? Don't we need to delete what the a pointer points to as well? Or is it because we deleted all the meaningful data that we don't need these (commented-out) parts of the destructor?

template<typename T> 
class mymatrix
 {
 private:
   struct ROW
   {
     T*  Cols;     // dynamic array of column elements
     int NumCols;  // total # of columns (0..NumCols-1)
   }; 

  ROW* Rows;     // dynamic array of ROWs
  int  NumRows;  // total # of rows (0..NumRows-1) 
}

Destructor:

virtual ~mymatrix()
{
for(int r=0;r<numRows;r++)
{
    for(int c=0;c<Rows[r].NumCols;c++)
    {
        delete Rows[r].Cols[c];
    }
    // delete Rows[r];
}
// delete Rows;
}

Constructor:

  mymatrix(int R, int C)
  {
    if (R < 1)
      throw invalid_argument("mymatrix::constructor: # of rows");
    if (C < 1)
      throw invalid_argument("mymatrix::constructor: # of cols");

    Rows = new ROW[R];  // an array with R ROW structs:
    NumRows = R;
    //intialize each row to C columns
    for(int r=0;r<R;r++){
        Rows[r].Cols=new T[C];
        Rows[r].NumCols=C;

        //initialize elements to their default value
        for(int c=0;c<Rows[r].NumCols;c++){
            Rows[r].Cols[c]=T{}; // default value for type T;
        }
    }
  }

Answer 1

You need to use array-delete syntax because you are deleting arrays, not single objects:

delete[] Rows[r].Cols

...

delete[] Rows

Edit: I originally simply included the correct delete[] operator usage and left everything unchanged in my original example for brevity, but as @idclev463035818 pointed out, whenever you define your own destructor, copy constructor, or copy assignment operator (especially when they involve dynamic memory allocation), you almost always need to have all three. Almost never do you want any one without the others because if you have raw pointers in your object, then they will be shallow-copied to the new objects being instantiated. Then later on, the destructors for each of these objects will be called and attempt to delete the same portions of memory multiple times, which is a major error. I've added these to the code example and make use of them in new tests in the main function.

---

Full code:

#include <iostream>
using namespace std;

template<typename T>
class mymatrix
{
public:
    struct ROW
    {
        T*  Cols;     // dynamic array of column elements
        int NumCols;  // total # of columns (0..NumCols-1)
    };

    ROW* Rows;     // dynamic array of ROWs
    int  NumRows;  // total # of rows (0..NumRows-1) 
public:
    mymatrix(int R, int C)
    {
        init(R, C);
    }

    void init(int R, int C) {
        if (R < 1)
            throw "";//throw invalid_argument("mymatrix::constructor: # of rows");
        if (C < 1)
            throw "";//invalid_argument("mymatrix::constructor: # of cols");

        Rows = new ROW[R];  // an array with R ROW structs:
        NumRows = R;
        //intialize each row to C columns
        for (int r = 0; r < R; r++) {
            Rows[r].Cols = new T[C];
            Rows[r].NumCols = C;

            //initialize elements to their default value
            for (int c = 0; c < Rows[r].NumCols; c++) {
                Rows[r].Cols[c] = T{}; // default value for type T;
            }
        }
    }

    mymatrix(const mymatrix& other) : mymatrix(other.NumRows, other.Rows[0].NumCols) {
        for (int r = 0; r < NumRows; ++r) {
            ROW& thisRow = Rows[r];
            ROW& otherRow = other.Rows[r];
            for (int c = 0; c < thisRow.NumCols; ++c) {
                thisRow.Cols[c] = otherRow.Cols[c];
            }
        }
    }

    mymatrix& operator=(const mymatrix& other) {
        if (other.NumRows != NumRows || other.Rows[0].NumCols != Rows[0].NumCols) {
            clear();
            init(other.NumRows, other.Rows[0].NumCols);
        }

        for (int r = 0; r < NumRows; ++r) {
            ROW& thisRow = Rows[r];
            ROW& otherRow = other.Rows[r];
            for (int c = 0; c < thisRow.NumCols; ++c) {
                thisRow.Cols[c] = otherRow.Cols[c];
            }
        }

        return *this;
    }

    void clear() {
        for (int r = 0; r < NumRows; r++)
        {
            delete[] Rows[r].Cols;
        }
        delete[] Rows;

        Rows = NULL;
        NumRows = 0;
    }

    virtual ~mymatrix()
    {
        clear();
    }

};

int main() {
    mymatrix<int> mat(5, 5);
    mat.Rows[0].Cols[2] = 5;

    mymatrix<int> matClone(mat);
    cout << matClone.Rows[0].Cols[2] << endl;

    matClone.Rows[0].Cols[1] = 8;

    cout << mat.Rows[0].Cols[1] << endl;

    mat = matClone;

    cout << mat.Rows[0].Cols[1] << endl;

    system("pause");
    return 0;
}

Answer 2

I'm telling this every time I see the usage of new/delete in C++.

Please don't use new/delete in C++!

Here is a minimal example of how to create a matrix which will allocate and delete itself without any manual memory allocations.

#include <vector>

class Matrix : public std::vector<std::vector<int>> {
public:
  Matrix(size_t numRows, size_t numCols)
      : std::vector<std::vector<int>>(numRows, std::vector<int>(numCols, 0)) {}
  size_t numRows() const { return size(); }
  size_t numCols() const { return front().size(); }
};

int main() {
  auto m = Matrix(5, 4);
  return m[4][3];
}