0

I'm working on some basic object practice and I faced this problem.

This class is named Matrix, and I tried to initialize a7 to a2.mutiply(a5).

But the result is quite different when I tried to intialize a7 at first, and assign it to a2.mutiply(a5) later.

I thought this two codes are the same, but the result told me I was wrong.

Can someone help me to point out the problem?

Matrix.h

// This is part of Martax.h codes.

class Matrix {
private:
    double ** data;     
    int row;            
    int col;
public:
    Matrix();  
    Matrix(int, int);  
    Matrix(int, int, double[], int);
    Matrix(const Matrix &); 

    ~Matrix();

    Matrix add(const Matrix &); 
    Matrix multiply(const Matrix &);    
    Matrix transpose();
    void displayData();
};

Matrix::Matrix()
{
    row = 2;
    col = 2;

    data = new double*[2];
    for (int i = 0; i < 2; i++) 
        data[i] = new double[2]{0};
}

Matrix::Matrix(int a ,int b)
{
    row = a;
    col = b;
    data = new double*[row];
    for (int i = 0; i < row; i++)
        data[i] = new double[col]{0};
}

Matrix::Matrix(int a, int b, double c[], int d) 
{
    row = a;
    col = b;
    data = new double*[row];
    int w = 0;
    for (int i = 0; i < row; i++)
        data[i] = new double[col]{0};

    for (int i = 0; i < row; i++) 
        for (int j = 0; j < col; j++)
        {
            if (w<d)
                data[i][j] = c[w];

            w++;
        }
}

Matrix::Matrix(const Matrix &a)
{
    row = a.row;
    col = a.col;
    data = new double*[row];
    for (int i = 0; i < row; i++)
        data[i] = new double[col];

    for (int i = 0; i < row; i++)
        for (int j = 0; j < col; j++)
            data[i][j] = a.data[i][j];
}

Matrix::~Matrix()
{
    for (int i = 0; i < row; i++)
        delete[] data[i];

    delete[] data;
}

Matrix Matrix::add(const Matrix &a)
{
    static Matrix a5(row,col);
    for (int i = 0; i < row; i++)
        for (int j = 0; j < col; j++)
            a5.data[i][j]=data[i][j] + a.data[i][j];

    return  a5;
}
Matrix Matrix::multiply(const Matrix &a)
{
    double *z;
    z = new double[row*a.col]{0};
    int w = 0;
    int u = 0;
    for (int i = 0; i<row; i++) 
    {
        while (a.col>u)
        {
            for (int j = 0; j < col; j++)
                z[w]+=(data[i][j])*(a.data[j][u]);  

            w++;
            u++;
        }
        u = 0;
    }

    return  Matrix(row,a.col, z, row*a.col);          
}
Matrix Matrix::transpose()
{
    double *z;
    z = new double[row*col];
    int w = 0;
    for (int j = 0; j< col; j++)
        for (int i = 0 ; i < row; i++)
        {
            z[w] = data[i][j];
            w++;
        }

    return  Matrix(col,row,z,row*col);
}
void Matrix::displayData() 
{
    for (int i = 0; i < row; i++) 
    {
        for (int j = 0; j < col; j++) 
           cout << setw(3) << data[i][j];
        cout  << endl;
    }
}

Source.cpp

#include<iostream>
#include "Matrix.h"
using namespace std;
int main()
{
    double c[4] = { 1,2,3,4 };
    Matrix a2(2, 2, c, 4);

    Matrix a5(2, 4, d, 8);

    Matrix a7 = a2.multiply(a5);


    cout << "a2 -->";
    a2.displayData();

    cout << "a5 -->";
    a5.displayData();

    cout << "a7 -->";
    a7.displayData();

    a7 = a2.multiply(a5);
    cout << "a7 -->";
    a7.displayData();

    system("pause");
    return 0;
}

Result.txt

"Source.cpp.exe"
Process started (PID=12908) >>>
a2 -->row = 2, column = 2
  1  2
  3  4
a5 -->row = 2, column = 4
  1  2  1  2
  1  2  1  2
a7 -->row = 2, column = 4
  3  6  3  6
  7 14  7 14
a7 -->row = 2, column = 4
<<< Process finished (PID=12908). (Exit code -1073741819)
================ READY ================
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
niorix
  • 65
  • 6
  • 2
    Read about [The Rule of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). – molbdnilo Mar 31 '20 at 16:22
  • To add a bit to the comment about the rule of three: `Matrix` doesn't define an assignment operator, so the compiler generates one. The compiler-generated assignment operator copies the `data` pointer, so you have two objects with the same pointer. Each of those objects will clean up that data in their destructor. When the second destructor runs, bad things will happen. – Pete Becker Mar 31 '20 at 16:25

1 Answers1

0

The program has undefined behavior because you did not define explicitly the copy assignment (or move assignment) operator.

After this statement

a7 = a2.multiply(a5);

the temporary object created by the expression a2.multiply(a5) will be destroyed. So the operators delete will be called for pointers copies of which are assigned to the object a7. Moreover there is a memory leak because the original pointers of the object a7 was not deleted.

You need to define explicitly the copy assignment operator or the move assignment operator or both.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • Umm, I know that `Matrix a1(a2)` is a copy constructor, but is `Matrix a7 = a2.mutiply(a5)` a copy constructor, too? I think this one will create a temporary object and assign it to `a7`, and lead to the same result (temporary object destructed), but why it won't happen? – niorix Apr 01 '20 at 06:51
  • @JstMonika When an object is created then the copy constructor is called. When an object is already created then the copy assignment operator is called. – Vlad from Moscow Apr 01 '20 at 09:16