0

The following code gives me the wrong output. actually, it is not doing sum, it actually copies the 2nd object to the M3 object instead of calculating the sum. I think I have some logical errors in + operator overloading. Does anybody have any idea or any other suggestion? it displays the output actually which is called in the copy constructor function cout<data[r][c]<<"\t";. but it did not display output when I use M3.displayData().

#include <iostream> 
#include <string.h>
using namespace std;

class Matrix{
    private:
        int noOfRows;
        int noOfColumns;
        int **data;
    public:
        Matrix(int noOfRows, int noOfColumns);
        void displayData();
        ~Matrix();
        Matrix (const Matrix &ref);
        Matrix operator + (Matrix m);
        Matrix& operator=(Matrix m) { 
        std::swap(m.noOfRows, noOfRows); 
        std::swap(m.noOfColumns, noOfColumns); 
        std::swap(m.data, data); 
        return *this; }
};

Matrix::Matrix(int inr=0, int inc=0){
    noOfRows=inr; noOfColumns=inc;
    data=new int*[noOfColumns];
    for(int i=0;i<noOfRows;i++)
        data[i]=new int[noOfColumns];
    int d;
    for(int r=0;r<noOfRows;r++){
        for(int c=0;c<noOfColumns;c++){
            cout<<"Enter ...";cin>>d;
            data[r][c]=d;
        }
        cout<<endl;
    }
}

Matrix::Matrix (const Matrix &ref){
    this->data=new int*[ref.noOfColumns];
    for(int i=0;i<ref.noOfRows;i++)
        this->data[i]=new int[ref.noOfRows];
    
    for(int r=0;r<ref.noOfRows;r++){
        for(int c=0;c<ref.noOfColumns;c++){
            this->data[r][c]=ref.data[r][c];
            cout<<this->data[r][c]<<"\t";
        }
        cout<<endl;
    }
}

Matrix Matrix::operator + (Matrix m){
    Matrix ms(m.noOfRows,m.noOfColumns);
    ms=0;
    for (int i=0; i<m.noOfRows; i++) 
        for (int j=0; j<m.noOfColumns; j++){
        ms.data[i][j] = data[i][j]+m.data[i][j];
        return ms;
        } 
    }

void Matrix::displayData(){
    for(int r=0;r<noOfRows;r++){
        for(int c=0;c<noOfColumns;c++)
            cout<<data[r][c]<<"\t";
        cout<<endl;
    }
}

Matrix::~Matrix(){
    delete[] data;
}

int main(){
    Matrix M1(2,2),M2(2,2);
    cout<<"\n Matrix A="<<endl;
    M1.displayData();
    cout<<"\n Matrix B="<<endl;
    M2.displayData();
    cout<<"\n Sum of Matrix="<<endl;
    Matrix M3=M1+M2;
    M3.displayData();
    return 0;
}

Output is here:

  • Error message seems quite clear. A `friend` is *not* a member. Take out the `Matrix::` from your definition of `Sum`. – Adrian Mole Mar 27 '22 at 16:19
  • `Matrix operator + (Matrix m);` -- Even if you got your code to compile, this will fail miserably. Read up on the [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three), and especially read the **Managing resources** section. I know you got an answer for the compiler error, but it will be fruitless until you fix this issue. You are missing an assignment operator, thus you only implemented 2 of the 3 required functions for copy safety. – PaulMcKenzie Mar 27 '22 at 16:32
  • `Matrix& operator=(Matrix m) { std::swap(m.noOfRows, noOfRows); std::swap(m.noOfColumns, noOfColumns); std::swap(m.data, data); return *this; }` -- That is basically what you're missing. – PaulMcKenzie Mar 27 '22 at 16:41
  • kindly copy past the code with changings, I can't understand :( – BS e-Learning Mar 27 '22 at 18:18
  • Literally take that code in the comment, and paste it into your class declaration. – PaulMcKenzie Mar 27 '22 at 18:28

2 Answers2

1

I'm not a matrix specialist, but I understood that matrix summation required both matrixes to be of the same size, and each elements to be summed up.

So you need to fully redefine operator+ (to avoid introducing exceptions here, I'd taken a permissive mathematical view, taking the max size of both matrixes and considering elements out of bounds to be 0):

Matrix Matrix::operator + (Matrix m){
    Matrix ms(max(noOfRows,m.noOfRows), max(noOfColumns+m.noOfColumns));
    for (int i=0; i<ms.noOfRows; i++) 
        for (int j=0; j<ms.noOfColumns; j++) 
           ms.data[i][j] = (i<noOfRows&&j<noOfColumns ? data[i][j]:0.0)
                             + (i<m.noOfRows&&j<m.noOfColumns ? m.data[i][j]:0.0);
    return ms;
}

By the way, it'll be safer to use the signature Matrix operator + (const Matrix& m) const, to avoid that a typo could accidentally change the value of the matrix, and avoid an unnecessary copy of the matrix argument.

Then, you must make Sum() a free standing function instead of a member function, if you want to call it like you do in main().

Christophe
  • 68,716
  • 7
  • 72
  • 138
0

The problem is that you have declared Sum as a friend function of class Matrix while defined it as a member function.

To solve the mentioned error just remove the Matrix:: qualification while defining it as shown below:

//----v-------------------------->removed `Matrix::` from here
Matrix Sum(Matrix m1,Matrix m2){
    Matrix m;
    m=m1+m2;
    return m;       
}

Also, the program may have other logical errors. You can refer to the rule of three for more information about them or ask a separate question if that doesn't help.

Jason
  • 36,170
  • 5
  • 26
  • 60
  • Give a mention that the code will fail anyway, even if it compiles successfully. This will eliminate the need for the OP to say "I made the changes, but the code doesn't work". – PaulMcKenzie Mar 27 '22 at 16:35
  • @PaulMcKenzie Yes, i have added a note saying that there may be other logical errors in OP's code and a link to the rule of three. – Jason Mar 27 '22 at 16:42