Nektar Nektar - 22 days ago 9
C++ Question

Matrix class implementation with operator overloading

I am trying to implement a Matrix class with operator overloading. The code can create matrix and also assign value to the indexes. But if I try to assign matrix one to another or apply any operation, the program crashes. I know I am missing something. But to me it seems my logic is correct and failing to find where is the wrong!

#include<iostream>
#include <cstdlib>
#include<cassert>
using namespace std;

class Matrix
{
public:
// constructor
Matrix(int rows,int cols)
{
if(rows>0 && cols >0){
Matdata = new double*[rows];

for (int i = 0; i <rows; i++)
{
Matdata[i] = new double[cols];
for(int j=0;j <cols;j++)
{
Matdata[i][j]=0;
}
}
}
}
// constructor
Matrix(int rows,int cols, double initial)
{
if(rows>0 && cols >0){
Matdata = new double*[rows];

for (int i = 0; i <rows; i++)
{
Matdata[i] = new double[cols];
for(int j=0;j <cols;j++)
Matdata[i][j] = initial;
}
}
}

//destructor
~Matrix(){
delete [] Matdata;
}

//access/subscript operator overload

double &operator()(int r, int c)
{
if(r<rows && c<cols);
return Matdata[r][c];
}
double &operator()(int r, int c) const
{
if(r<rows && c<cols);
return Matdata[r][c];
}


////copy constructor or assignment task
Matrix(const Matrix& rhs) {
rows = rhs.rows;
cols = rhs.cols;
Matdata = new double*[rhs.rows];
for (int i = 0; i <rows; i++)
{
Matdata[i] = new double[cols];
for(int j=0;j <cols;j++)
{
Matdata[i][j] = rhs.Matdata[i][j];
}
}
}

//// assignment operator for assigning a value from one Mat to another
Matrix& operator=(const Matrix& rhs) {

if (&rhs == this)
return *this;
Matrix temp(rhs);
std::swap(temp.Matdata, Matdata);
std::swap(temp.rows, rows);
std::swap(temp.cols, cols);
return *this;
}

//matrix addition implementation

Matrix& operator +=(const Matrix& rhs)
{ if(this->rows == rhs.rows && this->cols == rhs.cols){
rows = rhs.rows;
cols = rhs.cols;
for (int i=0; i<rows; i++) {
for (int j=0; j<cols; j++) {
this->Matdata[i][j] += rhs.Matdata[i][j];
}
}
return *this;
}
}

Matrix operator+(const Matrix& rhs){

if(this->rows == rhs.rows && this->cols == rhs.cols)
return (*this+= rhs);


}


Matrix operator*(const Matrix& rhs){
if(this->cols != rhs.rows)
{
return *this;
}
else{
rows = rhs.rows;
cols = rhs.cols;
Matrix result(rows, cols, 0.0);
for (int i=0; i<rows; i++) {
for (int j=0; j<cols; j++) {
for (int k=0; k<rows; k++) {
result.Matdata[i][j]+= this->Matdata[i][k] * rhs.Matdata[k][j];
}
}
}

return result;
}
}


private:
double **Matdata;
int rows;
int cols;
};


int main()
{
Matrix A = Matrix(2,2,1);
Matrix B = Matrix(2,2,5);
Matrix C = Matrix(2,2);


C = A+B; //fails

C(0,0) =112;
A(0,0) = B(0,0);

cout<< A(1,2)<<endl;
A = A+B; //fails
B = A; //fails
cout<< A(1,2)<<endl;
return 0;

}

Answer

I started to debug and see so many problems, that I only give you the first ones:

1) Your constructors did not initialize the variables of the class!

    Matrix(int rows,int cols, double initial):
        rows(rows),cols(cols)   << missing

2) Forget to return value:

    Matrix operator+(const Matrix& rhs){

        if(this->rows == rhs.rows && this->cols == rhs.cols)
            return (*this+= rhs);

        // And what if not??? Do we give nothing? bad idea!

    }

3 ) your operator+ calls operator +=

That is simply not what users of the class will expect. Why should

C = A+B;

modify A in this case.

4) Forget to return value in double &operator()(int r, int c)

5) Forget to return value in double &operator()(int r, int c) const

6) Out of bounds access with:

cout<< A(1,2)<<endl;

Matrix is of size 2x2 and you want to access the 3rd element! Counting here is 0..1 and 2 is out of bounds. Because problem 4) 5) you get nothing back from your functions and result is undefined!

7) Destruction of class leaks memory.

You forget to free the inner vars of your two dimensional array!

...1000)

Try to use your own debugger

There are at minimum 8 failures ( constructor problem is two times valid ) in around 100 lines of code. In addition a lot of code duplications and useless comments. Why a constructor have a comment "constructor"? And in general no kind of error handling. Returning references to data inside of an object is very bad design! What is if the object is destructed? The reference can be still in use. Accessing this dangling reference will cause undefined behaviour. Many many more! Do you want a rating :-)

Comments