C++ overloading operator+ for matrixes

Question

class matrix
{
    int n;
    double **a;  //the "matrix"
public:
    matrix(int);
    ~matrix();
    int getN();
    matrix& operator=(matrix&);
    double& operator()(int,int);
    friend matrix& operator+(matrix,matrix);
    friend matrix& operator-(matrix,matrix);
    friend matrix& operator*(matrix,matrix);
    friend ostream& operator<<(ostream &,const matrix &);
};

matrix& operator+(matrix A,matrix B)
{
    int i,j,n=A.getN();
    assert(A.getN()==B.getN());
    matrix *C=new matrix(A.getN());
    for(i=0; i<n; i++)
    {
        for(j=0; j<n; j++)
        {
            (*C)(i,j)=A(i,j)+B(i,j);
        }
    }
    return *C;
}

Is this the correct way to overload arithmetic operators?

Are there memory leaks in my code?

The constructor allocates memory in heap, first for an array of double pointers, then for each pointer an array of double.

For overload operator in general, you should read this : http://stackoverflow.com/questions/4421706/operator-overloading — BAK, Nov 13 '12 at 11:59

juanchopanza · Accepted Answer · 2012-11-13T11:53:53.930

6

You should be returning the new matrix by value, and passing (at leas one of) the arguments by const reference:

matrix operator+(const matrix& A, const matrix& B);

This means that you should not allocate it dynamically inside the body of the operator.

If you only call public member methods or member operators, then there is no need to declare the non-member operators as friend.

Also note that is is common practice to implement +=, *= etc. as member operators, and then implement the non-members in terms of those:

matrix operator+(matrix A, const matrix& B)
{
  return A+=B;
}

As an aside, you have to check that the dimensions of the matrices are correct. With your design it is only possible to do this at run-time. Another approach is to enforce dimensional correctness at compile time by making the matrices class templates:

template <typename T, size_t ROWS, size_t COLS> matrix;

The trade-off is that matrices of different dimensions are different types.

edited Nov 13 '12 at 11:53

answered Nov 13 '12 at 11:47

juanchopanza

223,364
34
402
480

1

@Cristi it should really be a const reference, `matrix& operator=(const matrix&);`, since assigning a RHS to a LHS shouldn't change the RHS! – juanchopanza Nov 13 '12 at 11:55
While this design is simple, it is inefficient as soon as you want to deal with more than 2 matrices, or if you have compound expressions (eg. C = 0.1*A + 0.2*B). This is due to the compiler generating lots of temporary matrices. There is also the problem of the compiler using the copy constructor to copy resulting matrices into an existing matrix object, meaning that an unnecessary copy is occurring (and hence slowing things down). Lastly, having sizes as template arguments leads to all sorts of messy problems later. For solutions, have a look at the source for [Armadillo](http://arma.sf.net) – mtall Nov 14 '12 at 08:59
@mtall Some of the copies you worry about get elided, and I didn't think OP would be ready for template metaprogramming. Concerning the size as a template parameter, I have used that technique widely, and I have found no problems, only advantages. I am not sure what messy problems you are referring to, but I only see having the size information at compile time as an advantage. – juanchopanza Nov 14 '12 at 09:07

score 1 · Answer 2 · answered Nov 13 '12 at 11:55

That sort of design will quickly be inefficient. Think of what happens when you do:

matrix D = A + B + C ;

Template meta programming gives you some really nice possibilities to avoid making useless temporaries. Armadillo is a really good implementation of the idea.

C++ overloading operator+ for matrixes

2 Answers2