CODEWITHSUNDEEP
c++templatesoperator-overloadingoperator-keyword
Tyler Jandreau
Tyler Jandreau

Reputation: 4335

Overloading "+" operator, wrong result?

I have a custom class which is designed to service a 2d array. I have overloaded the + operator but am getting some strange results that I didn't anticipate. I have the copy and assignment constructors here:

Array<T, ROW, COL>& operator=(const Array<T, ROW, COL> &rhs) {
    if (this != &rhs) {
        // allocate new memory
        T *newData = new T[ROW * COL];
        for (int i = 0; i < ROW; i++) {
            for (int j = 0; j < COL; j++) {
                newData[j*ROW + i] = rhs(i, j);
            }
        }
        data = newData;
    }
    return *this;
}

and here is my overloaded + operator:

inline Array<T, ROW, COL> &operator+(const Array<T, ROW, COL> &rhs) {
    for (int i = 0; i < ROW; i++) {
        for (int j = 0; j < COL; j++) {
            this->data[j*ROW + i] += rhs(i,j);
        }
    }
    return *this;
}

Here is a section of main:

Array<double, 10> ten;
ten.fill(10.0); // fill with tens
Array<double, 10> result = ten + ten + ten + ten;
std::cout << result << std::endl;

Yeilds:

[0]: 80 
[1]: 80 
[2]: 80 
[3]: 80 
[4]: 80 
[5]: 80 
[6]: 80 
[7]: 80 
[8]: 80 
[9]: 80

Which doesn't make sense to me. I would think that the result would be 40 all the way around. I have the copy and assignment constructors defined if ya'll need to see them.

What am I not understanding? Thanks!

Upvotes: 3

Views: 150

Answers (1)

Luchian Grigore
Luchian Grigore

Reputation: 258618

Don't, just don't. operator+ shouldn't modify the internal state of either of its operands. The correct prototype would be 

Array<T, ROW, COL> operator+(Array<T, ROW, COL> lhs, const Array<T, ROW, COL> &rhs)
{
    //logic goes here
    return lhs;
}

As you can see, you return by value and modify neither of the original parameters (the first one is passed by value, but since you're creating either a copy of it or a new object inside the function to return it, passing it by value is just as good - alternatively you can pass it by const reference). If you must keep it as a member (the one I wrote is the free operator), prototype it as

Array<T, ROW, COL> Array::operator+(const Array<T, ROW, COL> &rhs) const;

note the const qualifier, which tells you (and the reader) that this isn't modified.

EDIT: Ok, finally found the link - read this

Upvotes: 13

Related Questions