Destructor immediately calls and deletes my arrays

Question

I am working on a Matrix class for a CS project, and I'm trying to work on the constructors. The project calls for two different constructors, one just calling out the numbers of rows and columns and making them all 0 and another using an initializer list to assign the values. The header file so far is:

typedef unsigned int uint;
typedef std::initializer_list<std::initializer_list<double>> i_list;

class Matrix {
public:
double ** arr;
uint mainRows;
uint mainCols;

Matrix(uint rows, uint cols);
Matrix(const i_list & list);
Matrix(const Matrix & m);
~Matrix();

};

Some of the test cases require you to both define the rows and use the initializer list, for example:

Matrix d(2,2);
d = {{1,2},{3,4}};

But I noticed that every time I try and run this kind of code, the destructor will immediately delete the double ** arr which is where the values for the Matrix's are stored. Here is the code for the constructors:

    Matrix::Matrix(uint rows, uint cols)
{
    mainRows = rows;
    mainCols = cols;
    arr = new double*[rows];
    for (int i = 0; i < mainRows; i++) {
        arr[i] = new double[cols];
    }
    for (int i = 0; i < mainRows; i++) {
        for (int j = 0; j < mainCols; j++) {
            arr[i][j] = 0;
        }
    }
}

Matrix::Matrix(const i_list & list)
{
    int i = 0, j = 0;
    mainRows = list.size();
    mainCols = (*list.begin()).size();
    arr = new double*[mainRows];
    for (std::initializer_list<double> I : list) {
        j = 0;
        arr[i] = new double[mainCols];
        for (double d : I) {
            arr[i][j] = d;
            j++;
        }
        i++;
    }
}

Matrix::Matrix(const Matrix & m)
{
    this->arr = m.arr;
    this->mainRows = m.mainRows;
    this->mainCols = m.mainCols;
    for (uint i = 0; i < mainRows; i++) {
        for (uint j = 0; j < mainCols; j++) {
            this->arr[i][j] = m.arr[i][j];
        }
    }
}

Matrix::~Matrix()
{
    for (uint i = 0; i < mainRows; i++) {
        delete[] arr[i];
    }
    delete[] arr; 
}

I guess since its calling a constructor for the same object twice it's creating two double ** ars and that's why the Destructor want's to delete the original, but then I can't call on the values for other functions. Can somebody help me out with what I'm doing wrong?

Please create a [Minimal, **Complete**, and Verifiable Example](http://stackoverflow.com/help/mcve) to show us. I also recommend you take some time to [read about how to ask good questions](http://stackoverflow.com/help/how-to-ask). And of course read [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) by Eric Lippert, and learn how to use a debugger. — Some programmer dude, Oct 16 '17 at 15:27
Dupe of [this](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) if you did not. — NathanOliver, Oct 16 '17 at 15:27

score 3 · Accepted Answer · answered Oct 16 '17 at 15:35

The problem is that your copy-constructor only copies the pointer of the source object, not allocates new memory.

This is problematic because

d = {{1,2},{3,4}};

creates a temporary object out of {{1,2},{3,4}}. You statement is actually equal to

d = Matrix({{1,2},{3,4}});

which is equal to

d.operator=(Matrix({{1,2},{3,4}}));

After the assignment is made, you have two objects pointing to the same memory for arr. And then the temporary object is destructed, leading to arr inside d to become invalid, as it no longer points to allocated memory.

The naive solution is simple: Allocate memory for arr to point to in the copy-constructor. The better solution is to stop using pointers and dynamic allocation, and instead use std::vector, and live by the rule of zero, where you don't need any copy-constructor, no copy-assignment operator and no destructor.

score 1 · Answer 2 · answered Oct 16 '17 at 15:36

This is wrong:

Matrix::Matrix(const Matrix & m)
{
    this->arr = m.arr;
    this->mainRows = m.mainRows;
    this->mainCols = m.mainCols;
    for (uint i = 0; i < mainRows; i++) {
        for (uint j = 0; j < mainCols; j++) {
            this->arr[i][j] = m.arr[i][j];
        }
    }
}

Note yuo do not creating actual copy here. this->arr = m.arr; makes both pointers to pointing same part of memory so new and old instance of Matrix are sharing this memory. So flowing for loops does nothing.

Than when one of instances is destroyed the other instance is pointing to memory which is freed.

Destructor immediately calls and deletes my arrays

2 Answers2