Trouble in a OOP C++ operator

Question

I am quite new to C++. My problem is to write an OOP C++ program about matrix (create a matrix class and add some methods to this class) Here is my code:

#include <iostream>
#include <iomanip>

using namespace std;

class Matrix 
{
    private:
        int col, row;
        double *a;
    public:
        Matrix(int col = 1, int row = 1) {
            this->col = col; this->row = row;
        }
        ~Matrix() {
            delete a;
            col = row = 0;
        }
        void insertMatrix() {
            a = new double[this->col * this->row];
            for (int i = 0; i < this->row; i++)
                for (int j = 0; j < this->col; j++) {
                    cout << endl << "a[" << i + 1 << "][" << j + 1 << "] = ";
                    cin >> this->a[i * this->col + j];
                }
        }
        void printMatrix() {
            for (int i = 0; i < this->row; i++) {
                for (int j = 0; j < this->col; j++)
                    cout << setw(9) << this->a[i * this->col + j];
                cout << endl;
            }
            cout << endl;
        }
        int getCol() {
            return col;
        }
        int getRow() {
            return row;
        }
        Matrix operator+(Matrix);
        Matrix operator-(Matrix);
};

Matrix Matrix::operator+(Matrix x) {
    if (x.col != col || x.row != row) {
        cout << endl << "Can't add these two matrices";
        exit(0);
    }
    Matrix sum(x.col, x.row);
    sum.a = new double(sum.col * sum.row);
    for (int i = 0; i < this->col * this->row; i++)
        sum.a[i] = a[i] + x.a[i];
    return sum;
}

Matrix Matrix::operator-(Matrix x) {
    if (x.col != this->col || x.row != this->row) {
        cout << endl << "Can't subtract these two matrices";
        exit(0);
    }
    Matrix dif(this->col, this->row);
    dif.a = new double(dif.col * dif.row);
    for (int i = 0; i < this->col * this->row; i++)
        dif.a[i] = this->a[i] - x.a[i];
    return dif;
}

int main()
{
    int row, col;
    cout << endl << "Column = "; cin >> col; cout << endl << "Row = "; cin >> row;
    Matrix A(col, row), B(col, row);
    A.insertMatrix(); B.insertMatrix();
    cout << "Matrix A: " << endl; A.printMatrix(); 
    cout << "Matrix B: " << endl; B.printMatrix();
    cout << "Matrix (A + B)" << endl; (A + B).printMatrix();
    cout << "Matrix (A - B)" << endl; (A - B).printMatrix();
}

I don't see any mistake at all. I can compile the program. But every time I try to input some numbers, the program keeps freezing with the message "stop working" and gets the wrong answer. I use Orwell Dev C++ in Windows 8. Can anyone explain why?

Answer 1

One mistake is that you're using new incorrectly.

 sum.a = new double(sum.col * sum.row);

The above creates a single double dynamically, and initializes the value to sum.col * sum.row.

You should be using new[]:

 sum.a = new double[sum.col * sum.row];

Then you must use delete[], not delete when using new[].

Another error is that you did not initialize the a pointer in your constructor. When the Matrix is destroyed, the destructor will issue a delete on a pointer that is pointing to who-knows-where.

Thus, this simple one line program has problems:

int main() {
   Matrix m;
}  // < -- problems here 

The fix is to make sure your pointers are initialized:

Matrix(int mcol = 1, int mrow = 1) : a(0), col(mcol), row(mrow) {}

Note that a is now initialized to 0. Also note the usage of the member initialization list.

However, another mistake is that your Matrix class lacks a user defined copy constructor and assignment operator. When you return or pass a Matrix by value as you're doing here:

Matrix Matrix::operator+(Matrix x)

The copy will only do a shallow copy on the pointers, and your program thus will have memory leaks, double deallocation errors, etc. Please read up on the "rule of 3":

What is The Rule of Three?

Other issues:

Do not call exit(0) within class code. If I wanted to use your Matrix class, and I give operator + a Matrix that isn't the correct size, please don't close the application down on me! Instead, throw an exception, or issue an assert().

See here: exit() call inside a function which should return a reference

Answer 2

The main problem in your program is that in the operators you overload, you return a stack allocated Matrix object which will call the copy constructor.

But since you don't define one, the compiler will generate it for you, doing a bit-wise copy, which means that the d pointer in the copied object will point to the same location as in the object from stack.

Therefore, the destructor is called two times

• when leaving the function overloads;
• when destroying the copied object, which happens immediately after this line:

(A + B).printMatrix();

That is why your program will terminate abnormally with an error similar to this:

Error in `./rez': free(): invalid next size (fast): 0x00000000020390c0

As others suggest, you wouldn't have encountered this problem if you had respected The rule of 3 (or 5 in C++11/14).

Answer 3

Another mistake is a blatant violation of the Rule Of Three; copying your Matrix ruins its ownership semantics.

Why don't you use a std::vector<double> instead.