10

Background: I have a complicated class with many variables. I have a sound and tested copy constructor:

Applepie::Applepie( const Applepie &copy) :
m_crust(copy.m_crust),
m_filling(copy.m_filling)
{
}

Some of the member variable copy constructors called in the intializer list perform allocation.

Question: I need to create operator=. Rather than duplicating the existing constuctor with assignment instead of initialization list, and freeing memory that's being replaced, and etc etc etc, can I simply do the following:

Applepie& Applepie::operator=( const Applepie &copy)
{
    if( this != &copy)
    {
       this->~Applepie(); // release own object
       new(this) Applepie(copy); // placement new copy constructor
    }
    return *this;
}

In other words, is destroy self followed by a placement new copy constructor semantically identical to operator= ?

This seems to have the potential to dramatically reduce repeat code and confirming that each variable is initialized properly, at the cost of potential slight loss of efficiency during assignment. Am I missing something more obscure?

Rationale: My actual class has about 30 varaibles. I am concerned about the fact that both my copy constructor and my assignment operator have to copy all thirty, and that the code might diverge, causing the two operations to do things differently.

jcwenger
  • 11,254
  • 1
  • 50
  • 63
  • 4
    If the copy ctor throws, you have broken the object, so you're not giving any expection safety guarantees. – R. Martinho Fernandes Aug 24 '11 at 15:18
  • 1
    @R Martinho -- It seems to me that any situation in which the copy ctor would throw would also cause my operator= to throw if I was manually assigning each variable... So... that still seems equivalent? – jcwenger Aug 24 '11 at 15:21
  • 3
    the problem is not `operator=` throwing, is `operator=` leaving the object in an invalid state! It's been destroyed. – R. Martinho Fernandes Aug 24 '11 at 15:22
  • 3
    Does apple pie really need 30 member variables? Does *any* class need 30 member variables? Doubtful. Can some of those members be combined into sensible classes? Likely. – Benjamin Lindley Aug 24 '11 at 15:43
  • jcwenger: The copy and swap idium uses the copy cnstructor but will not leave the current object in an invalid state if the copy constructor throws. In your version you have lost the object whatever happnes so an exception will cause you to have an invalid object in the system (this is not desirable). There shouldne **zero** code that provides no gurantee. – Martin York Aug 24 '11 at 15:46
  • There is a potential extra cost in destroying and recreating all the members. Often some members can reuse existing resources when assigned to. – Bo Persson Aug 24 '11 at 15:53
  • @jcwenger: if your member objects are well designed, you shouldn't need to manually write a copy constructor at all, which solves your problem quite easily. – Mooing Duck Mar 02 '12 at 18:28
  • Reference also to: http://stackoverflow.com/questions/1734628/copy-constructor-and-operator-overload-in-c-is-a-common-function-possible – jcwenger Jul 31 '15 at 14:29

2 Answers2

6

As Herb Sutter in "Exceptional C++" states, it is not exception safe. That means, if anything is going wrong during new or construction of the new object, the left hand operand of the assignment is in bad (undefined) state, calling for more trouble. I would strongly recommend using the copy & swap idiom.

Applepie& Applepie::operator=(Applepie copy)
{
  swap(m_crust, copy.m_crust);
  swap(m_filling, copy.m_filling);
  return *this;
}

When your object uses the Pimpl idiom (pointer to implementation) also, the swap is done by changing only two pointers.

Community
  • 1
  • 1
René Richter
  • 3,512
  • 2
  • 27
  • 37
  • 2
    +1 `swap` is a wonderful undervalued function... Additional advantage: the compiler can perform copy elision if the argument is an rvalue. – David Rodríguez - dribeas Aug 24 '11 at 15:32
  • So... My issue with this is that my actual class has about 30 variables. My copy constructor has all 30 variables in the initializer list. copy&swap will still require me to have 30 swap() calls -- my object is to reduce the likelihood that Applepie pie(original); and Applepie apple; apple = original; produce different results. – jcwenger Aug 24 '11 at 15:37
  • @jcwenger: Yes, that's a call for trouble. So it might be a candidate for the *Pimpl* idiom, but before switching to that, consider it is coming at a cost: each member access is going through a pointer then. Another idea would be to split your `Applepie` class into smaller pieces. If you have 30 data members in one class, something might be wrong with the design... – René Richter Aug 24 '11 at 15:45
  • 3
    @jcwenger: I would say your real objective is to write code that is hard to break. The copy and swap idiom does this. I think you are trying to compensate for other problems (like 30 members). To be effecient your object should already have a swap() method. Thus the assignment operator simply becomes a call to copy constructor followed by a call to swap(). – Martin York Aug 24 '11 at 15:49
  • 2
    It is hard without seeing the code, but if your class is managing resources (else compiler generated copy constructor and assignment would work) and you have more than 30 fields... that is a bit of a code smell. Consider revising the design to split responsibilities up. – David Rodríguez - dribeas Aug 24 '11 at 16:06
  • But [std::swap is **not** guaranteed to be exception-free either](https://stackoverflow.com/a/14601648/566849). – Fabio A. Oct 28 '19 at 08:54
1

In addition to Rene's answer, there is also the problem of what would happen if ApplePie was a base class of the actual object: ApplePie would be replacing the object with an object of the wrong type!

SSJ_GZ
  • 753
  • 1
  • 5
  • 15