calling copy constructor in assignment operator

Question

In an already existing class of a project I am working on I encountered some strange piece of code: The assignment operator calls the copy constructor.

I added some code and now the assignment operator seems to cause trouble. It is working fine though if I just use the assignment operator generated by the compiler instead. So I found a solution, but I'm still curious to find out the reason why this isn't working.

Since the original code is thousands of lines I created a simpler example for you to look at.

#include <iostream>
#include <vector>

class Example {

private:
  int pValue;
public:
  Example(int iValue=0)
  {
    pValue = iValue;
  }

  Example(const Example &eSource)
  {
    pValue = eSource.pValue;
  }

  Example operator= (const Example &eSource)
  {
    Example tmp(eSource);
    return tmp;
  }

  int getValue()
  {
    return pValue;
  }

};

int main ()
{
  std::vector<Example> myvector;

  for (int i=1; i<=8; i++) myvector.push_back(Example(i));

  std::cout << "myvector contains:";
  for (unsigned i=0; i<myvector.size(); ++i)
    std::cout << ' ' << myvector[i].getValue();
  std::cout << '\n';

  myvector.erase (myvector.begin(),myvector.begin()+3);

  std::cout << "myvector contains:";
  for (unsigned i=0; i<myvector.size(); ++i)
    std::cout << ' ' << myvector[i].getValue();
  std::cout << '\n';

  return 0;
}

The output is

myvector contains: 1 2 3 4 5

but it should be (an in fact is, if I just use the compiler-generated assignment operator)

myvector contains: 4 5 6 7 8

Answer 1

Your operator= does not do what everyone (including the standard library) thinks it should be doing. It doesn't modify *this at all - it just creates a new copy and returns it.

It's normal to re-use the copy constructor in the copy assignment operator using the copy-and-swap idiom:

Example& operator= (Example eSource)
{
  swap(eSource);
  return *this;
}

Notice how the operator takes its parameter by value. This means the copy-constructor will be called to construct the parameter, and you can then just swap with that copy, effectively assigning to *this.

Also note that it's expected from operator= to return by reference; when overloading operators, always follow the expected conventions. Even more, the standard actually requires the assignment operator of a CopyAssignable or MoveAssignable type to return a non-const reference (C++11 [moveassignable] and [copyassignable]); so to correctly use the class with the standard library, it has to comply.

Of course, it requires you to implement a swap() function in your class:

void swap(Example &other)
{
  using std::swap;
  swap(pValue, other.pValue);
}

The function should not raise exceptions (thanks @JamesKanze for mentioning this), no to compromise the exception safety of the operator=.

Also note that you should use the compiler-generated default constructors and assignment operators whenever you can; they can never get out of sync with the class's contents. In your case, there's no reason to provide the custom ones (but I assume the class is a simplified version for posting here).