Question

I was doing this as a personal exercise and wanted to make sure I got this right and understand it correctly. I have a coordinate class with members row and column. I wanted to overload the + and += operator. Here is my code:

Coordinate& Coordinate :: operator+= (const Coordinate& rhs){
    this->m_Row += rhs.m_Row;
    this->m_Column += rhs.m_Column;

    return *this;
}

Coordinate& operator+ (const Coordinate& lhs, const Coordinate& rhs) {
    return Coordinate(lhs) += rhs;
}

where

friend Coordinate& operator + (const Coordinate& lhs, const Coordinate& rhs);

is a friend function defined in Coordinate class.

Are there any pitfalls to this code ?

Here is my understanding of how these work:

operator += 

Add rhs m_Row and m_Column to this members. Return a reference to the object pointed by this pointer and thereby avoid creating another object due to copy constructor.

operator +

Create a local object of lhs (since lhs is a constant and we don't want to modify its contents) using the copy constructor (lets call it localObj). Invoke the += member operator on localObj which performs the addition. Return a reference to this localObj so that we don't create another object due to copy constructor, otherwise.

Now, the very last statement concerns me, since I am returning a reference to a local object. As soon as the function (operator +) goes out of scope, localObj will be destroyed and returned reference will point to an object which has been destroyed. Am I correct in understanding this ?

If so, how should I fix it ??

EDIT: After all the answers and what I learnt: here is what my Coordinate class looks like now: http://rextester.com/MJJI7394

Was it helpful?

Solution

You are right to be worried, you are returning a reference to a temporary here:

Coordinate& operator+ (const Coordinate& lhs, const Coordinate& rhs) {
    return Coordinate(lhs) += rhs;
}

You need to return a Coordinate by value, for example like this:

Coordinate operator+ (Coordinate lhs, const Coordinate& rhs) {
    return lhs += rhs;
}

In the example above, we make a copy of the first parameter instead of taking a reference and then copying in the body of the function. Then we return the result of += on that, by value.

With this setup, there is no need to declare operator+ as a friend.

See this SO link for more information, and thanks to @Blastfurnace for pointing it out.

OTHER TIPS

Personally, I would define operator+=() in terms of operator+() and operator=():

Coordinate operator+(const Coordinate& lhs, const Coordinate& rhs) {
  return Coordinate(lhs.getRow() + rhs.getRow(), lhs.getCol() + rhs.getCol();
}

const Coordinate& operator=(Coordinate& lhs, const Coordinate& rhs) {
  lhs.setRow(rhs.getRow());
  lhs.setCol(rhs.setCol());

  return lhs;
}

const Coordinate& operator+=(Coordinate& lhs, const Coordinate&rhs) {
  return lhs = lhs + rhs;
}

I'm using setters and getters here. Alternatively, you can use friend and/or member functions. Note that all functions that return references are returning the parameters that are sent in so that there are no issues with references to local or temporary objects.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top