CPP: матричный деструктор

Question

Для следующего определения класса mymatrix, почему мне не нужны закомментированные части деструктора? Разве нам не нужно удалять то, на что указывает указатель? Или потому, что мы удалили все значимые данные, которые нам не нужны (закомментированные) части деструктора?

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) 
}

Деструктор:

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;
}

Конструктор:

  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

Я говорю это каждый раз, когда вижу использование new/delete в C++.

Пожалуйста, не используйте new/delete в C++!

Вот минимальный пример того, как создать матрицу, которая будет выделять и удалять себя без каких-либо ручных выделений памяти.

#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];
}

Answer 2

Вам необходимо использовать синтаксис удаления массива, поскольку вы удаляете массивы, а не отдельные объекты:

delete[] Rows[r].Cols

...

delete[] Rows

Редактировать: Первоначально я просто включил правильное использование оператора delete[] и оставил все без изменений в моем исходном примере для краткости, но, как указал @ idclev463035818, всякий раз, когда вы определяете свой собственный деструктор, конструктор копирования или оператор назначения копирования ( особенно когда они включают динамическое распределение памяти c), вам почти всегда нужно иметь все три. Почти никогда вы не хотите никого без других, потому что если у вас есть необработанные указатели в вашем объекте, то они будут скопированы на мелкие экземпляры новых объектов. Затем позже будут вызваны деструкторы для каждого из этих объектов, которые попытаются удалить одни и те же части памяти несколько раз, что является серьезной ошибкой. Я добавил их в пример кода и использую их в новых тестах в функции main.

---

Полный код:

#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;
}