-1

I am trying to write a matrice class in c++ with a overloaded + operator that is a friend function for summing a matrix with a number, ie sum every element of the matrice with the number.

for example:-

2+ |1|2|  = |3|4|
   |2|1|    |4|3|

main.cpp :-

#include <iostream>
using namespace std;


class matrice {
    int** a;
    int rows, columns, i, j;
public:
    matrice() {}
    matrice(matrice& A) { //copy constructor for deep copying the result of 2+A into C
        rows = A.rows;
        columns = A.columns;
        a = new int* [rows];
        for (i = 0; i < rows; i++) {
            a[i] = new int[columns];
            *a[i] = 0;
        }
        for (i = 0; i < rows; i++) {
            for (j = 0; j < columns; j++) {
                a[i][j] = A.a[i][j];
            }
        }
    }
    matrice(int m, int n) {
        rows = m;
        columns = n;
        a = new int* [rows];
        for (i = 0; i < rows; i++) {
            a[i] = new int[columns];
            *a[i] = 0;
        }

    }
    ~matrice() {
        delete[] a;
    }
    void insert(int p, int q, int value) {
        a[p][q] = value;
    }

    void print() {
        for (i = 0; i < rows; i++) {
            cout << "\n";
            for (j = 0; j < columns; j++) {
                cout << a[i][j] << " ";
            }

        }
    }
    friend matrice operator+(int k, matrice& A);
    friend matrice operator+(matrice& A, int k);
};
matrice operator+(int k, matrice& A) { //for performing 2+A
    matrice temp(A.rows, A.columns);
    for (int i = 0; i < A.rows; i++) {
        for (int j = 0; j < A.columns; j++) {
            temp.a[i][j] = A.a[i][j] + k;
        }
    }
    return temp;
}

matrice operator+(matrice& A, int k) {  //for performing A+2
    matrice temp(A.rows, A.columns);
    for (int i = 0; i < A.rows; i++) {
        for (int j = 0; j < A.columns; j++) {
            temp.a[i][j] = A.a[i][j] + k;
        }
    }
    return temp;
}


int main() {
    int i, j, m, n, value;
    cout << "\nEnter order of A matrice:";
    cin >> m >> n;
    matrice A(m, n);
    cout << "\nEnter the matrice:";
    for (i = 0; i < m; i++) {
        cout << "\nEnter row " << i + 1 << " : ";
        for (j = 0; j < n; j++) {
            cin >> value;
            A.insert(i, j, value);
        }
    }
    cout << "\nThe entered matrice is :";
    A.print();
    cout << "\n\n";
    cout << "\nEnter order of B matrice:";
    cin >> m >> n;
    matrice B(m, n);
    cout << "\nEnter the matrice:";
    for (i = 0; i < m; i++) {
        cout << "\nEnter row " << i + 1 << " : ";
        for (j = 0; j < n; j++) {
            cin >> value;
            B.insert(i, j, value);
        }
    }
    cout << "\nThe entered matrice is :";
    B.print();
    cout << "\n\ntesting 2+A"; 
    matrice C; //Everything upto here is fine
    C = A + 2; // C.a is pointing to unreadable memory because of destruction after doing A+2 
    C.print(); // so access violation error
}

The problem is that the destructor of C is called after invoking the copy constructor which causes the double pointer a in C to be deleted. So when C.print() is called An Access violation reading location 0xDDDDDDDD exception is thrown.

I can't find out why C's destructor is called.

alvin lal
  • 138
  • 4
  • 10
  • `*a[i] = 0;` you don't need this assignment in the copy constructor, you're assigning all the elements below – bloody Dec 06 '20 at 10:51
  • 1
    don't repeat yourself: `matrice operator+(int k, matrice& A) { return A+k; }` – JHBonarius Dec 06 '20 at 10:52
  • 4
    there's a big memory leak in you destructor... You're only deleting the `a[]` and not all its elements/arrays... But there's a big problem with who's owning what data in your code. Think it out in a design before you start to code. – JHBonarius Dec 06 '20 at 10:54
  • 1
    related: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three – 463035818_is_not_an_ai Dec 06 '20 at 10:58
  • an `int**` isnt much better than a `std::vector>` if you do not want to use a vector for learning purpose you should still store the matrix in a flat array, memory wise it will be more efficient and using a `int*` will also simplify the memory managment – 463035818_is_not_an_ai Dec 06 '20 at 11:04
  • @bloody you are correct, thanks for the point ! – alvin lal Dec 06 '20 at 11:27
  • @JHBonarius then it is showing error "class matrice has no suitable copy constructor" – alvin lal Dec 06 '20 at 11:29
  • That's because you're not following the [Rule of Five](https://stackoverflow.com/a/4782927) – JHBonarius Dec 06 '20 at 11:31
  • @largest_prime_is_463035818 whoa, that's from 2010 ;) it's rule of five in the mean time – JHBonarius Dec 06 '20 at 11:32
  • @largest_prime_is_463035818 vector would be a better choice , but i can't use vector in this particular case. Even if we store the array as a flat array in int* instead of int** , wouldn't the destructor be called and hence, won't the array be deleted? – alvin lal Dec 06 '20 at 11:33
  • @JHBonarius answers of course mention the rule of 5 (and the rule of 0). I mean if someone needs to read that Q&A unlikely they will get confused by the missing 2 in the question ;) – 463035818_is_not_an_ai Dec 06 '20 at 11:34

1 Answers1

3

The rule of 5 says that if you manually handle allocations, you should have an explicit (or explicitely deleted):

  • copy constructor
  • copy assignment operator
  • move constructor
  • move assignement operator
  • destructor

You might omit the ones that you are sure that you code will never use, but it is highly dangerous.

Anyway correct allocation/de-allocation handling is complex and requires very cautious code. For example the correct declaration for a copy ctor should be matrice(const matrice& A), and comments already warned you of memory leaks.

So my advice is:

  • if you want to learn about manual allocation management, follow best practices: rule of 5 an copy/swap idiom to avoid caveats
  • if you just want working code, replace 1D dynamic arrays with vectors and let the standard library handle the corner cases
Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252