need help to overload the operator +

Question

so i want to overload operator + (C++) in my class matrices to use this instruction m1 = m2 + m3

knowing that i have already overload th operator = but it crashes at execution here is my code class matrices:

#include "../include/matrices.h"
#include <iostream>
using namespace std;

class matrices
{
    public:
        int taille;
        int **tab;
    public:
        matrices();
        matrices(int);
        matrices(matrices &);
        virtual ~matrices();
        void setTaille(int);
        int getTaille();
        void setVal(int, int, int);
        int getVal(int, int);
        friend istream& operator >> (istream&, matrices& ); //surcharge de l'opérateur >>
        friend ostream& operator << (ostream&, const matrices& ); //surcharge de l'opérateur <<
        //void operator=(matrices&);
        matrices& operator=(matrices& );
        //friend matrices& operator+(matrices , matrices );
        friend matrices& operator+(matrices&, matrices&);
        friend matrices operator-(matrices);
        friend matrices operator*(matrices);
};

matrices::~matrices()
{
    //dtor
}

matrices::matrices()
{
    taille = 2;
    this->tab = new int*[taille];
    for(int i=0; i<taille; i++)
        this ->tab[i] = new int[taille];
}

matrices::matrices(int n)
{
    taille = n;
    this->tab = new int*[taille];
    for(int i=0; i<taille; i++)
        this->tab[i] = new int[taille];
}

matrices::matrices(matrices & m1)
{
    (*this).taille = m1.taille;
    tab = new int*[(*this).taille];
    for(int i=0; i<(*this).taille; i++)
        for(int j=0; j<(*this).taille; j++)
            this->tab[i][j] = m1.tab[i][j];
}

void matrices::setTaille(int n)
{
    (*this).taille = n;
}

int matrices::getTaille()
{
    return (*this).taille;
}

void matrices::setVal(int val, int n, int m)
{
    this->tab[n][m] = val;
}

int matrices::getVal(int n, int m)
{
    return this->tab[n][m];
}

istream& operator >> (istream& flux, matrices& m1)
{
    for(int i=0; i<m1.taille; i++)
    {
        for(int j=0; j<m1.taille; j++)
        {
            flux >> m1.tab[i][j];
        }
    }
    return (flux);
}

ostream& operator << (ostream& flux, const matrices& m1)
{
    for(int i=0; i<m1.taille; i++)
    {
        for(int j=0; j<m1.taille; j++)
        {
            flux << m1.tab[i][j];
            cout << "\t" ;
        }
        cout << "\n" << endl;
    }
    return (flux);
}

matrices& matrices::operator=(matrices& m1)
{
    delete this->tab;
    (*this).taille = m1.taille;
    this->tab = new int*[m1.taille];
    for(int i=0; i<m1.taille; i++)
        this->tab[i] = new int[m1.taille];

    for(int i=0; i<m1.taille; i++)
    {
        for(int j=0; j<m1.taille; j++)
        {
            this->tab[i][j] = m1.tab[i][j];
        }
    }
    return (*this);
}

matrices& operator+(matrices&  m1, matrices&  m2)
{
    matrices mtmp;
    mtmp.taille = m1.taille;
    mtmp.tab = new int*[mtmp.taille];
    for(int i=0; i<mtmp.taille; i++)
        mtmp.tab[i] = new int[mtmp.taille];

        if(m2.taille == m1.taille)
        {
            for(int i=0; i<mtmp.taille; i++)
            {
                for(int j=0; j<mtmp.taille; j++)
                {
                    mtmp.tab[i][j] = m1.tab[i][j] + m2.tab[i][j];
                }
            }
        }
        return mtmp;
}

and this is my main fonction

#include <iostream>
#include "include/matrices.h"
#include<stdlib.h>
#include<stdio.h>
#include<string>

using namespace std;

int main()
{
    matrices m1,m2,m3;
    cin >> m1;
    cin >> m3;
    m2 = m1;
    cout << "=================================" << endl;
    cout << "==== Affichage de la matrice ====" << endl;
    cout << "=================================" << endl;
    cout << m2;
    getchar();

    cout << "==================================" << endl;
    cout << "==== sommation des 2 matrices ====" << endl;
    cout << "==================================" << endl;
    m3 = m1 + m2;
    cout << "==================================" << endl;
    cout << "==== matrice m3 matrices est: ====" << endl;
    cout << "==================================" << endl;
    cout << m3;
}

thanks for your help

Answer 1

You must be either using a compiler that does not give you a warning about this, or are ignoring the warning:

matrices& operator+(matrices&  m1, matrices&  m2)
{
    matrices mtmp;
    ...
    return mtmp;
}

The code is a big problem: you are attempting to return a reference to an automatic variable. By the time you attempt to use the reference, the object no longer exists.

You should declare your overload this way:

matrices operator+(const matrices&  m1, const matrices&  m2)
{
    matrices mtmp;
    ...
    return mtmp;
}

Which will also require you to change your friend declaration:

friend matrices operator+(const matrices&, const matrices&);

That way you are returning a copy (since you aren't changing m1 and m2, you should declare their references as const).

Additionally, you should look into the copy-swap idiom for your copy-assignment operator as doing something like m1 = m1 would cause a crash in your current code. As a corollary to this point: you do not declare a copy-constructor, so when you do something like matricies m2 = m1, it is doing a shallow copy (the default implementation of the copy-constructor). So m2's pointers are pointing to the data in m1. When you destruct m1, m2 contains dangling pointers ... which will cause a heap corruption (deleting already deleted memory).

A lot of your memory issues (and they are many) go away simply by switching to std::vector or std::array (depending on how you want to define your class) instead of trying to manage the memory yourself. A simple (incomplete) example: http://ideone.com/VVqvGy

Answer 2

There are quite a few problems with your class.

1. You dynamically allocate memory in the constructor, but you don't free it in the destructor. This is a memory leak.
2. In operator=, you free memory that you allocated with new[] with delete (notice the lack of[]): This is undefined behavior, and does random stuff. You should also delete all dynamically allocated memory, not just the 'top level' - the same way that you allocate it in a loop in the constructor, you should free it in a loop, and use delete[].
3. Your setTaille method is pointless: It will only destroy your data, lead to memory leaks and inconsistency in your data. It should be either completely reworked, to reallocate and copy the contents of the current matrix to allow the size to be set, or removed altogether. (I can honestly not imagine a use case for it)
4. in operator+, you should check that the size of the matrices are the same, before performing any allocations. You should make a decision what should the result of adding non-compatible matrices be (it's a tough choice).

The error in your code most probably comes from error number 2.

EDIT: I spotted another problem that I overlooked: In your matrices& operator+ function, you return a reference to a auto variable (that is, memory on the stack). This is illegal: The moment that you leave the function, this memory becomes garbage. You most probably want to return a copy of the constructed matrix(Use matrices operator+