Object of a class corrupted when calling a member function of the class

Question

A great deal of unexpected numbers outputted on my console window when I compiled and ran the program.

The class "matrix" is declared as below:

class matrix
{
public:
    vector<vector<int> > M;
    
    matrix();
    matrix(int m, int n);
    matrix(vector<vector<int> > &m);
    matrix Mul(matrix m1);
    void Inverse();
    bool SquareMatrix();
    void GetDet();
    int Det(vector<vector<int> > &m);
    void Print();
};

the member function "Mul":

matrix matrix::Mul(matrix m1)
{
    vector<vector<int> > Temp(M.size(), vector<int>(m1.M[0].size(), 0));

    if (M[0].size() != m1.M.size()) 
    {
        cout << "Cannot do multiplication!" << endl;
        return matrix();
    }
    else 
    {       
        for (int i = 0; i < M.size(); i++)
        {
            for (int j = 0; j < m1.M[0].size(); j++)
            {
                int ele_buf = 0;
                for (int k = 0; k < M[0].size(); k++)
                {
                    ele_buf += M[i][k] * m1.M[k][j];
                }
                Temp[i][j] = ele_buf;
            }
        }
    }

    int d1 = M.size(), d2 = m1.M[0].size();
    for (int i = 0; i < M.size(); i++)
    {
        M[i].clear();
    }
    M.clear();

    M.resize(Temp.size(), vector<int>(Temp[0].size(), 0));
    for (int i = 0; i < d1; i++)
    {
        for (int j = 0; j < d2; j++)
        {
            M[i][j] = Temp[i][j];
        }
    }
}

M[i][j] hold the expected value at this point.

the member function "Print":

void matrix::Print()
{
    for (int i = 0; i < M.size(); i++) {
        for (int j = 0; j < M[0].size(); j++) {
            cout << M[i][j] << " ";
        }
        cout << endl;
    }
}

Output wrong M.

main:

/*
call constructor to initialize m1 and m2
*/
//...
matrix m3 = m1.Mul(m2);
m3.Print();
//...

How can I fix it?

I'm new here, plz let me know if I didn't make my question clear.

Editted:

matrix matrix::Mul(matrix m1)
{
    vector<vector<int> > Temp(M.size(), vector<int>(m1.M[0].size(), 0));

    if (M[0].size() != m1.M.size()) 
    {
        cout << "Cannot do multiplication!" << endl;
        return matrix();
    }
    else 
    {
        //TO_DO: Multiply two matrixes and print the result.
        
        for (int i = 0; i < M.size(); i++)
        {
            for (int j = 0; j < m1.M[0].size(); j++)
            {
                int ele_buf = 0;
                for (int k = 0; k < M[0].size(); k++)
                {
                    ele_buf += M[i][k] * m1.M[k][j];
                }
                Temp[i][j] = ele_buf;
            }
        }
    }

    int d1 = M.size(), d2 = m1.M[0].size();
    for (int i = 0; i < M.size(); i++)
    {
        M[i].clear();
    }
    M.clear();

    return matrix(Temp);
}

Functionning properly, thanks.

Ah I see my problem. Sorry for asking such a stupid question@@ — SH Chen, Jul 30 '20 at 07:06

score 1 · Answer 1 · answered Jul 30 '20 at 07:07

You are promised to return matrix from the Mul function

matrix matrix::Mul(matrix m1);
^^^^^^

and you did not keep the promise. This can cause undefined behaviour to your program.

Add an proper return statement in the function.

matrix matrix::Mul(matrix m1)
{
  // ... code
  return *this; // matrix(Temp) itself
}

score 1 · Accepted Answer · answered Jul 30 '20 at 07:25

You forgot to add a return statement, which is trivial to fix. However, your code is not written in a good OO manner, so here come few suggestions for the improvement:

don't pass matrix m1 by value, because you are paying an expensive copy each time you invoke this function; use const matrix& m1 instead

in the body of Mul you are modifying member M, which is a serious side effect; remove the part of code:

int d1 = M.size(), d2 = m1.M[0].size();
for (int i = 0; i < M.size(); i++)
{
    M[i].clear();
}
M.clear();

instead of implementing a separate function called Mul, overload the * operator (see e.g. here and here)

Object of a class corrupted when calling a member function of the class

2 Answers2