Overloading operator + in c++ to add two arrays

Question

Task is to overload the operator + so that it adds two arrays. Array is a class with two private members int* data, and int m(capacity of array).

void Array::setM(int m){
this->m = m;
this->data = new int[this->m];
}

int& Array::operator[](int i){
return this->data[i];
}


Array& Array::operator+(Array& a){
Array res;
if (this->m >= a.m) {
    res.setM(this->m);
    for (int i=0; i<this->m; i++){
    res.data[i] = this->data[i] + a.data[i];
    }
}
else if (this->m < a.m) {
    res.setM(a.m);
    for (int i=0; i<a.m; i++){
    res.data[i] = this->data[i] + a.data[i];
    }
}
Array& rez = res;
return rez;
}

This is main:

int main()
{
Array a(3);
Array& a1 = a;
a1[0] = 1;
a1[1] = 2;
a1[2] = 4;

Array b(3);
Array& b1 = b;
b1[0] = 1;
b1[1] = 2;
b1[2] = 4;

Array& c = a1.operator+(b1);
for (int i=0; i<3; i++){
    cout<<c[i]<<" ";
}
return 0;
}

The function works fine when the return type is Array, but when the return type is Array& it returns 173104 4200736 4200896. I am starting with c++ so references confuse me a bit, I do not see a point of function returning the reference type?

`operator+` shouldn't return a reference. Just return a copy. — cigien, Aug 24 '20 at 19:25
More wisdom about operator overloads: [What are the basic rules and idioms for operator overloading?](https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading) — user4581301, Aug 24 '20 at 19:26
The kicker with a reference is the referred-to variable must outlive the need for the reference. — user4581301, Aug 24 '20 at 19:30
Yes, exactly, but the argument should be `const` as well as the function itself. Or even a free function `Array operator+(Array const &, Array const &)`. See the link @user4581301 showed. — cigien, Aug 24 '20 at 19:35

Vlad from Moscow · Accepted Answer · 2020-08-24T20:22:45.353

Let's consider your operator implementation step by step.

For starters the right hand operand is not changed within the operator. So the function should be declared at least like

Array& operator+( const Array& a );

The function crates a new object of the type Array within its body.

Array res;

So as the object is a local variable of the function you may not return a reference to it. It means that the operator should be declared like

Array operator+( const Array& a );

or

const Array operator+( const Array& a );

This code snippet

if (this->m >= a.m) {
    res.setM(this->m);
    for (int i=0; i<this->m; i++){
    res.data[i] = this->data[i] + a.data[i];
    }
}

can invoke undefined behavior when a.m is less than this->m.

As the member function does not change the left operand then the function should be a constant member function.

const Array operator +( const Array& a ) const;

Usually such operators are declared as separate friend functions of class.

friend const Array operator+( const Array &a, const Array &b );

Taking into account the algorithm that you are using within the operator it can be defined (outside the class definition. Or if you want to define it within the class definition then you need to use the specifier friend) the following way

const Array operator+( const Array &a, const Array &b )
{
    int m = std::min( a.m, b.m );
    Array res( m );

    for ( int i = 0; i < m; i++ )
    {
        res.data[i] = a.data[i] + b.data[i];
    }

    return res;
}

Pay attention to that the function setM can produce memory leaks because it does not delete early allocated memory for an object of the type Array

void Array::setM(int m){
this->m = m;
this->data = new int[this->m];
}

That is the function is unsafe.

Also bear in mind that the constructor with parameter should be declared with the function specifier explicit. Otherwise you can encounter unexpected conversions of an integer to the type Array.

As you are dynamically allocating arrays you need to write explicitly at least the destructor, copy constructor and copy assignment operator.

score 0 · Answer 2 · answered Aug 24 '20 at 19:41

You are breaking the rules here, you got what is called undefined behavior. Your program could crash with a segmentation fault, or it can output garbage, just like what you observe.

You see, you are returning a reference to a local variable. This is not good. Let me explain.

Variable that you declare inside a function usually have automatic storage. This means that they have a well defined beginning of existance (the declaration) and an end of existence which is the end of its scope. In your case this is the end of the function:

Array& Array::operator+(Array& a) {
    Array res;
    if (this->m >= a.m) {
        res.setM(this->m);

        for (int i=0; i<this->m; i++){
            res.data[i] = this->data[i] + a.data[i];
        }
    } else if (this->m < a.m) {
        res.setM(a.m);

        for (int i = 0; i < a.m; i++) {
            res.data[i] = this->data[i] + a.data[i];
        } // <-- here, `i` dies.
    }

    Array& rez = res;
    return rez;
} // <-- here, `res` dies.

So the reference rez point to a dead variable. Reading it is categorized as undefined behavior by the standard.

The obvious solution is to return by value, and you should also take a const reference to the argument:

 Array Array::operator+(Array const& a) {
    Array res;
    
    // [...]

    return res;
}

In this case the compiler will copy (or move) the res object. Keep in mind that compilers are really good at their job and will most likely apply copy elision in this particular case. This means that there will be no copy and the variable res will be constructed directly in the return channel.

Overloading operator + in c++ to add two arrays

2 Answers2