CODEWITHSUNDEEP
c++copy-constructor
DCEx1
DCEx1

Reputation: 21

Destructor causes the program to crash

I am really struggling with the concept of using destructors with copy constructors. If I don't use a destructor the code works fine, as it does it automatically. If I do, I get an error saying 'Debug Assertion Failed!' and 'Expression:_BLOCK_TYPE_IS_VALID(pHead->nBlockUse).

But I want to be able to understand how to use destructors. Here is the code below, I would very much appreciate help explaining what I have done incorrectly or need to do!
class Matrix {

private:
    int M;
    int N;
    double *data;

public:
    Matrix();
    int getM() const { return M; }
    int getN() const { return N; }

    //CONSTRUCTOR
    Matrix(int sizeR, int sizeC,double * input_data)
    {
        M = sizeR; //Rows
        N = sizeC; //Columns

        data = new double[M*N]; //creation of 1D array, uses m&n values

        cout << "\nMatrix::Matrix(int sizeR, int sizeC, double * data_value) is invoked...\n\n";

        //ENTER DATA INTO MATRIX HERE:
        for(int i=0; i < M*N; i++) //Loops for every data entry into 1D array, uses r&c as referenece to
            data[i] = input_data[i];//Accesses each value at specific location, inputs value 'val'
        for(int i = 0; i < M*N; i++) //Loops for every data entry into 1D array, uses r&c as referenece to size
            cout << data[i] << " ";
    }

    //get function uses row and column from user
    double get(int i, int j)
    {
        return data[i*N+j];
    }

    double set(int i, int j, double val)
    {
        data[i*N+j] = val;

        cout << "\n\nNEW MATRIX: ";
        for(int i = 0; i < M*N; i++)//Loops for every data entry into 1D array, uses r&c as referenece to size
            cout << data[i] << " ";

        return val;
    }

    Matrix(const Matrix& oldMatrix)
    {
        cout¸<< "\nMatrix::Matrix(const Matrix&) is invoked....";
        M = oldMatrix.getM();
        N = oldMatrix.getN();
        data = oldMatrix.data;

        cout << "\n\n";

        //ENTER DATA INTO MATRIX HERE:

        for(int i = 0; i < M*N; i++)//Loops for every data entry into 1D array, uses r&c as referenece to size
            cout << data[i] << " ";

    }

    //DESTRUCTOR
    ~Matrix()
    {
        //delete data
        delete [] data;
        data = NULL;
        cout << "\n\nMatrix::~Matrix() is invoked...\n\n";
    }



};

int main()
{
    int sizeR, sizeC;
    double val;

    cout << "Enter No. Rows: ";
    cin >> sizeR;

    cout << "Enter No. Columns: ";
    cin >> sizeC;

    double * input_data;


    input_data = new double[sizeR*sizeC];

    //INPUTS VALUES TO ARRAY
    for(int i = 0; i < sizeR*sizeC; i++)//Loops for every row
        input_data[i] = i;

    Matrix M1(sizeR, sizeC, input_data);

    cout << "Enter row that value you are after is in: ";
    cin >> sizeR;
    cout << " & now the column that it is in: ";
    cin >> sizeC;


    cout << "Change value: " << M1.get(sizeR, sizeC) << " to:";
    cin >> val;
    M1.set(sizeR, sizeC, val);

    //calls copy constructor
    M1 = Matrix(M1);
}

Upvotes: 0

Views: 1579

Answers (3)

Noel
Noel

Reputation: 770

A solution could be adding a bool variable (is_copy for instance) to your Matrix class. Set it to false on constructor and to true on copy constructor. Only deallocate memory in destructor if is_copy is false.

Or, as suggested in the comments, better use a smart pointer.

Upvotes: -2

Some programmer dude
Some programmer dude

Reputation: 409136

In the copy-constructor you copy the pointer, meaning you you now have two objects both having the same pointer. If one of those objects are destructed, then it leaves the other object with a now invalid pointer.

Dereferencing this pointer in anyway, or attempting to free it, will lead to undefined behavior.

The problematic line in question is this one:

M1 = Matrix(M1);

That line creates a temporary object, and copies the data from M1 into that temporary object, then it assigns the temporary object back to M1 (and the compiler-generated copy-assignment operator will just do a shallow copy of the members, so not much different from your copy-constructor) and then destruct the temporary object, leading to the stray and invalid pointer in M1.

On a slightly related issue, you also might want to learn about the rule of three.

Upvotes: 4

xMRi
xMRi

Reputation: 15355

You are copying a pointer of one object into another inside the Copy constructor:

Matrix(const Matrix& oldMatrix)
{
   ...
   data = oldMatrix.data;

After calling the copy constructor you have two objects referring to the same memoryblock. And if one object is destroyed the memory block is deleted and the second object points to an invalid memory location.

In the copy constructor you also need to allocate a new buffer!

Upvotes: 0

Related Questions