What is wrong in the operator overloading function in the matrix class

Question

I had to create this class to add two matrixes, using operator overloading. I don't want to use pointers in this problem because I don't understand them fully yet, I'm just trying to learn operator overloading. the error it shows is a segmentation fault (core dumped).

#include <cmath>
#include <cstdio>
#include <vector>
#include <iostream>
#include <algorithm>
using namespace std;

class Matrix {
    public : 
        vector<vector<int> >a;
        Matrix(){}
        Matrix operator+( Matrix m ){
            Matrix sum;
            for (int i=0; i<a.size(); i++ ){
                for(int j=0;j<a[i].size(); j++){
                    sum.a[i][j] = a[i][j] + m.a[i][j];
                }
            } 
            return sum;
        }
};
int main () {
   int cases,k;
   cin >> cases;
   for(k=0;k<cases;k++) {
      Matrix x;
      Matrix y;
      Matrix result;
      int n,m,i,j;
      cin >> n >> m;
      for(i=0;i<n;i++) {
         vector<int> b;
         int num;
         for(j=0;j<m;j++) {
            cin >> num;
            b.push_back(num);
         }
         x.a.push_back(b);
      }
      for(i=0;i<n;i++) {
         vector<int> b;
         int num;
         for(j=0;j<m;j++) {
            cin >> num;
            b.push_back(num);
         }
         y.a.push_back(b);
      }
      result = x+y;
      for(i=0;i<n;i++) {
         for(j=0;j<m;j++) {
            cout << result.a[i][j] << " ";
         }
         cout << endl;
      }
   }  
   return 0;
}

At first glance: your `Matrix sum.a` is empty, but you're trying to access its elements. — JohnFilleau, Mar 11 '20 at 21:43

score 1 · Answer 1 · answered Mar 11 '20 at 21:45

The vector inside your Matrix sum has zero size.
SO using operator[] will cause undefined behavior.

        Matrix sum;                                 // sum.a.size() is zero
        for (int i=0; i<a.size(); i++ ){
            for(int j=0;j<a[i].size(); j++){
                sum.a[i][j] = a[i][j] + m.a[i][j];  // broken
            }                                       // sum.a[0][0] does not exist
        }

There are a couple of solutions.
Easiest is to resize the array so that it has the required number of rows/cols

score 1 · Answer 2 · answered Mar 11 '20 at 21:45

1

You did not initialize the vector for the matrix defined on this line:

Matrix sum;

Your entire approach could use a rethink. Making the callers responsible for managing the internal representation of your matrix breaks the principle of encapsulation.

Furthermore, using vector<vector<int>> is inefficient for non-sparse matrix representations. You would be better off using a single block of contiguous memory.

Either way, I recommend defining a constructor for Matrix that accepts a size. Make the internal data private and define operator[] or similar to access the data

answered Mar 11 '20 at 21:45

paddy

60,864
6
61
103

1

Here is a very simple version of what Paddy means by *using a single block of contiguous memory*: https://stackoverflow.com/a/2076668/4581301 – user4581301 Mar 11 '20 at 21:50
Thank you. This code is not entirely mine. it was a problem on hackerrank and I had to build the class using vector and operator overloading. I will try to use the approach you suggested. could you suggest me a way to initialize a 2d vector? – ANIKET LAVKUSH VISHWAKARMA 19B Mar 11 '20 at 22:02
1

If you _must_, then `Matrix::Matrix(size_t size) : a(size, vector(size)) {}` – paddy Mar 11 '20 at 22:13
1

A side note, @ANIKETLAVKUSHVISHWAKARMA19B , programming competition sites are not the best places to learn to program. The automated judging systems care about only one thing: Is the output correct this one time. It doesn't care if it would be wrong next time. It does not care how many language rules were broken. It does not care if the program would fail utterly with a slightly different input set. Nor care if the program could be used to compromise the computer. It does not care if the code is readable or maintainable. It skips many details you have to worry about as a programmer. – user4581301 Mar 11 '20 at 22:59

score 1 · Answer 3 · answered Mar 11 '20 at 22:03

Inside the operator you created an object of the type Matrix that has an empty vector

Matrix sum;

So you may not use the subscript operator in the expression

sum.a[i][j]

You could after creating the object sum set appropriate sizes for the vector.

For example

Matrix sum;
sum.a.assign( a.size(), std::vector<int>( a[0].size() ) );

Pay attention to that it is much better to declare the operator the following way

Matrix operator+( const Matrix &m ) const;

because neither operand of the operator is changed in the operator.

What is wrong in the operator overloading function in the matrix class

3 Answers3