memory leak in overloading operator =

Question

I have this constructor for matrix to allocate memory

class Matrix 
{

public:
    int** matrix;
    int cols;
    int rows;
};
Matrix::Matrix(int row, int col)
{
    cols = col;
    rows = row;
    matrix = new int*[rows];
    int i;
    for (i = 0; i < rows; ++i)
    {
        matrix[i] = new int[cols];
    }
}

now I want to overload the operator =, but I can't figure out how to write the function and allocate the new memory, without getting a memory leak, or not having enough memory.

The matrix the I'll do the = on it, already has memory allocated for it, so can i delete the memory and make new memory on the size of the other one?

for now I have this on the operator =

this->rows = other.rows;
this->cols = other.cols;

int i, j;
for (i = 0; i < this->rows; ++i)
{
    for (j = 0; j < this->cols; j++)
    {
        this->matrix[i][j] = other.matrix[i][j];
    }

}
return *this;

Do you have a copy constructor? – Captain Giraffe Aug 24 '15 at 11:28 — Captain Giraffe, Aug 24 '15 at 11:28
yes, i did a copy constructor – Mumfordwiz Aug 24 '15 at 11:30 — Mumfordwiz, Aug 24 '15 at 11:30

score 5 · Accepted Answer · edited May 23 '17 at 10:32

5

The idiomatic way is to use the copy/swap idiom. See What is the copy-and-swap idiom?

The assignment is then reduced to

Matrix& operator=(Matrix copy){
   swap(*this, copy);
   return *this;
}

See the linked question for all the perks you gain by using this idiom.

edited May 23 '17 at 10:32

Community

1
1

answered Aug 24 '15 at 11:31

Captain Giraffe

13,403
5
35
65

score 3 · Answer 2 · answered Aug 24 '15 at 11:30

I would recommend switching from manually allocating arrays to using std::vector

class Matrix 
{
public:
    Matrix(int row, int col);
    int cols;
    int rows;
    std::vector<std::vector<int>> matrix;
};

Matrix::Matrix(int row, int col)
:  cols(col),
   rows(row),
   matrix(rows, std::vector<int>(cols))
{ }

Now you can let the compiler generate your copy assignment operator, and the other constructors, destructor, etc. This class is now copyable, moveable, and doesn't leak memory because matrix now uses RAII semantics instead of you having to manage its memory.

score 0 · Answer 3 · answered Aug 24 '15 at 11:36

First you can reallocate the each column using the delete operator.

for (i = 0; i < rows; ++i)
    {
           delete []matrix[i];
    }

Then you can deallocate pointers to each row.

 delete []matrix;

After that you can allocate the new matrix as required from the one passed as argument.

memory leak in overloading operator =

3 Answers3