c++ overloading operators, assignment, deep-copy and addition.

Question

I'm doing some exploration of operator-overloading at the moment whilst re-reading some of my old University text-books and I think I'm mis-understanding something, so hopefully this will be some nice easy reputation for some answerers. If this is a duplicate please point me in the right direction.

I've created a simple counter class that has (at this stage) a single member, val (an int).

I have initialised three of these counters, varOne to varThree, and want the third counter to be the sum of the first two (e.g. varThree.val is set to 5 in the below code)

counter::counter(int initialVal)
{
    val = initialVal;
    //pVal = new int;
    //*pVal = 10; // an arbitrary number for now
}

int main (int argc, char const* argv[])
{
    counter varOne(3), varTwo(2), varThree;
    varThree = varOne + varTwo;

    return 0;
}

I've overloaded operator+ like so:

counter operator+(counter& lhs, counter& rhs)
{
    counter temp(lhs.val + rhs.val);
    return temp;
}

I've made this a non-member function, and a friend of the counter class so that it can access the private values.

My problem starts when adding another private member, pVal (a pointer to an int). Adding this means that I can no longer do a simple varThree = varOne copy because when varOne is destroyed, varThree.pVal will still be pointing to the same bit of memory.

I've overloaded operator= as follows.

int counter::getN()
{
    return *newVal;
}

counter& counter::operator=(counter &rhs)
{
    if (this == &rhs) return *this;
    val = rhs.val;
    delete pVal;
    pVal = new int;
    *pVal = rhs.getN();
    return *this;
}

Now if I do something like varThree = varOne everything copies correctly, however trying to do varThree = varOne + varTwo gives me the following error:

counter.cpp: In function ‘int main(int, const char**)’:
counter.cpp:96: error: no match for ‘operator=’ in ‘varThree = operator+(counter&, counter&)(((counter&)(& varTwo)))’
counter.cpp:55: note: candidates are: counter& counter::operator=(counter&)
make: *** [counter] Error 1

It looks as though counter::operator= is having trouble coping with the return output from operator+, and that I need to overload operator= further to accept the type that operator+ is returning, but I've had no luck and I'm beginning to think that maybe I've done something fundamentally wrong.

Answer 1

You need to pass your parameters as const reference. For example:

counter& counter::operator=( const counter &rhs )

And similarly for operator+(). This is necessary in order to be able to bind temporary values to the function parameter(s). Temporary values are created when you return by value, so when you say:

varOne + varTwo

a nameless temporary is created. This is the right thing to do, but you have to make sure that functions such as the assignment op can accept such values by making their parameters const.

You also need to implement the copy constructor and destructor for your class, though lack of these will not cause compilation errors (unfortunately).

Answer 2

Another way to approach this problem is to use the PImpl pattern and swap for the assignment operator. Assuming that you still have a counter(int) constructor you could write operator= as follows

counter& counter::operator=(const counter& rhs) {
  counter temp(rhs.getN());
  std::swap(&pVal,rhs.pVal);
  return *this;
}

This has the benefit of leaving the messy memory management functions in the constructor and destructor where they should be.

Answer 3

The key here (as touched upon by a previous poster) but worth emphasizing is that expressions in C++ can be categorized as being either rvalues or lvalues.

Their is much detail behind those categories, but a useful heuristic to guide your intuition is: if you can take the address of an expression (such as a variable) it is an lvalue (there is much more to the story here, but this is a good place to start).

If it is truly not an lvalue, it is an rvalue - and a useful heuristic for rvalues is that they represent "hidden" temporary objects that the compiler instantiates to make your code work. These objects are created and destroyed by the compiler behind the scenes.

Why is this relevant here?

Well, in C++98/03 (which is what i presume you are using), remember the following two rules:

1) Only lvalue expressions can bind to non-const references (ignoring casts) 2) rvalue expressions can bind to only const references (ignoring casts)

An example will help here:

// Consider the function foo below - it returns an int - 
// whenever this function is called, the compiler has
// to behave as if a temporary int object with the value 5 is returned.
// The use of 'foo()' is an expression that is an rvalue - try typing &foo() -
// [Note: if foo was declared as int& foo(), the story gets complicated, so
//   i'll leave that for another post if someone asks]

int foo() { return 5; }

void bind_r(int& r) { return; }
void bind_cr(const int& cr) { return; }

int main()
{
   int i = 10;  // ok
   int& ri = i; // ok binding lvalue to non-const reference, see rule #1
   int& ri2 = foo(); // Not ok, binding a temporary (rvalue) to a non-const reference 
        // The temporary int is created & destroyed by compiler here

   const int& cri = foo(); // ok - see rule #2, temporary int is NOT destroyed here

  //Similarly
   bind_r(i); // ok - rule #1
   bind_r(foo()); // NOT ok - rule #2
   bind_cr(foo()); // ok - rule #2


  // Since the rules above keep you out of trouble, but do not exhaust all possibilities
  // know that the following is well-formed too:
  const int& cri2 = i;
  bind_cr(i);
  bind_cr(cri);
  bind_cr(cri2); 

}

When you bind an rvalue to a const reference, you basically extend the temporary object's life-time to the life-time (in this case scope) of the reference (and the compiler can not just destroy it at the end of that expression) - so you end up with a reference to a valid object.

I hope this helps in understanding why you have to declare your assignment operator as accepting a const reference and not just a non-const reference, as one of the other posters rightly recommended.

p.s. There are some other issues with your code (such as why you are destroying and creating the memory that your object exclusively points to upon each assignment, and the lack of a copy constructor and destructor), and if no one has addressed them by the time this post appears, i will :)

p.s. It may also be worth knowing that C++0x adds something known as rvalue references (non-const) that preferentially bind to rvalues and provide for extremely powerful optimization opportunities for the programmer (without having to rely on the optimization capabilities of the compiler) - they also help solve the problem of creating the perfect forwarding function in C++ - but now we're digressing ;)