constructor\destructor or understanding of OOP

Question

Im trying to to use operator overloading with both + and = operator on a matrix class which i created. either the constructor or destructor is causing a problem or it isn't either (though i grayed out each of them and both and the code seems to work). could someone please help me understand what is causing this strange behavior. when i try to create 3 matrixes a b and c then try a = b+c; it just fails.

header file
#ifndef MATRIX_H;
#define MATRIX_H;
using namespace std;


enter code here

class matrix
{
friend ostream& operator<< (ostream&, matrix &);
public:
    matrix();
    matrix(int,int); //constructor
    matrix(const matrix&);//copy constructor
    ~matrix();
    int getRow();
    int getCol();
    void setRow(int);
    void setCol(int);
    class arr1D{                //proxy class to allow the use of [][]       operator
        public:
        arr1D(int* a):temp(a){}
        int &operator[](int a){
            return temp[a];
        }
        int *temp;
    };
    arr1D operator[](int a){
    return arr1D(arr2[a]);
    }
    matrix& operator=(const matrix& );

    matrix& operator+(const matrix& );

protected:
private:
    int row , col;
    int **arr2;

   };

   #endif // MATRIX_H

enter code here
cpp file
#include <iostream>
#include "matrix.h"

using namespace std;

matrix::matrix()
{
setCol(0);
setRow(0);
**arr2=0;
}


matrix::matrix(int x, int y) //matrix constructor creates x*y matrix and                 initializes to 0's
{
setCol(y);
setRow(x);
arr2 = new int * [getRow()];
if (arr2) {
    for (int i = 0; i < getRow(); i++) {
        arr2[i] = new int [getCol()];

    };
};
for (int i=0; i<getRow();i++){
    for (int j=0;j<getCol();j++){
        arr2[i][j]=0;
    };
};
}

matrix::matrix(const matrix &m){ //defines the copying constructor
 row=m.row;
 col=m.col;
arr2 = new int*[row];
for (int i=0; i<row; i++){
    arr2[i] = new int[col];
    }
for (int i=0; i<row; i++){
    for (int j=0; j<col; j++){
        arr2[i][j] = m.arr2[i][j];
    }
 }

}

 matrix::~matrix(){ //defines the destructor
 for (int i=0; i<row; i++){
    delete[] arr2[i];
    }
delete[] arr2;
}


int matrix::getRow(){ //getter for row
return row;
}

int matrix::getCol(){ // getter for col
return col;
}

void matrix::setRow(int x){ //setter for row
row=x;
}

void matrix::setCol(int x){ //setter for col
col=x;
 }


ostream& operator<< (ostream& output, matrix& a){
     int i,j;
     for (i=0; i < a.getRow() ; i++){
        for (j=0; j< a.getCol() ; j++){

            output << " " <<a.arr2[i][j];

        };
         output << "\n";
     };
return output;
}

matrix& matrix::operator=(const matrix& right)
{
if (this == &right) {     // Same object?
  return *this;
}
row = right.row;
col = right.col;
for (int i=0; i<row; i++)
{
  for (int j=0; j<col; j++){
    arr2[i][j]=right.arr2[i][j];
  }
}
return *this ;
}

matrix& matrix::operator+(const matrix& right)
{
int row=right.row;
int col=right.col;
matrix result(row,col);
for (int i = 0; i < row; i++){
    for (int j = 0; j < col; j++){
            //cout<<"arr2[i][j]="<<arr2[i][j]<<endl;
            //cout<<"right.arr2[i][j]="<<right.arr2[i][j]<<endl;
        result.arr2[i][j]=(arr2[i][j] + right.arr2[i][j]);
            //cout<<"result.arr2[i][j]="<<result.arr2[i][j]<<endl;
    };
 };
 return result;
 }

Answer 1

First, as the other answer pointed out, you are returning a reference to a temporary in your operator +. That is undefined behavior.

But instead of writing operator + in this fashion, what you should do is write operator += instead, and then in turn, write operator + in terms of operator +=. Since a programmer would expect += to also work for the matrix in addition to +, it makes no sense to leave out +=.

For operator +=, you would in this case return a reference to the current object.

So all we need to do is move the code in operator + to operator +=:

#include <exception>
//...
matrix& matrix::operator+=(const matrix& right)
{
    if(row != right.row || col != right.col)
       throw std::logic_error("Matrix not the same size");

    for (int i = 0; i < right.row; i++)
    {
        for (int j = 0; j < right.col; j++)
             arr2[i][j] += right.arr2[i][j]);
    }
    return *this;
 }

Note that we return a reference to the current matrix, since += modifies the current matrix. Also note that we throw an exception on an illegal matrix being sent to +=. This IMO makes more sense than returning a legitimate matrix back on error. If the matrix is not the same size, the code should not try and return a matrix back.

Now operator + can be written in terms of +=:

   matrix matrix::operator+(const matrix& right)
   {
      return matrix(*this) += right;
   }

Believe it or not, that's it. All we did was create a temporary matrix and call += with the passed in argument. We return the result of this as a brand new matrix, just as we expect.

The other issue is the assignment operator. Given that you've written a copy constructor and destructor, and the copy constructor works without having to use the assignment operator, then the copy / swap idiom can be used to implement the assignment operator.

   #include <algorithm>
   //...
   matrix& matrix::operator=(const matrix& right)
   {
      matrix temp(right);
      std::swap(temp.arr2, arr2);
      std::swap(temp.row, row);
      std::swap(temp.col. col);
      return *this;
   }

All we did here was create a temporary matrix of the passed in object and swap out its contents with the current object's contents. When the temporary dies off at the return, the temporary destroys the old contents that were swapped out.

This method has the advantage of not only being very simple to implement (just a bunch of calls to std::swap), it is also exception safe. If there is an issue creating the temporary matrix, a std::bad_alloc exception would have been thrown, without messing up or altering any members of this. The issue with the other answer given, where you're deallocating the memory first before allocating the new memory, is solved by using the above technique.