Implement copy-ctor in terms of copy-operator or separately?

Question

This is not a duplicate of Implementing the copy constructor in terms of operator= but is a more specific question. (Or so I like to think.)

Intro

Given a (hypothetical) class like this:

struct FooBar {
  long id;
  double valX;
  double valZ;
  long   valN;
  bool   flag; 
  NonCopyable implementation_detail; // cannot and must not be copied

  // ...
};

we cannot copy this by the default generated functions, because you can neither copy construct nor copy a NonCopyable object. However, this part of the object is an implementation detail we are actually not interested in copying.

It does also does not make any sense to write a swap function for this, because the swap function could just replicate what std::swap does (minus the NonCopyable).

So if we want to copy these objects, we are left with implementing the copy-ctor and copy-operator ourselves. This is trivially done by just assigning the other members.

Question

If we need to implement copy ctor and operator, should we implement the copy ctor in terms of the copy operator, or should we "duplicate" the code with initialization list?

That is, given:

FooBar& operator=(FooBar const& rhs) {
  // no self assignment check necessary
  id = rhs.id;
  valX = rhs.valX;
  valZ = rhs.valZ;
  valN = rhs.valN;
  flag = rhs.flag;
  // don't copy implementation_detail
  return *this;
}

Should we write a)

FooBar(FooBar const& rhs) {
  *this = rhs;
}

or b)

FooBar(FooBar const& rhs)
: id(rhs.id)
, valX(rhs.valX)
, valZ(rhs.valZ)
, valN(rhs.valN)
, flag(rhs.flag)
// don't copy implementation_detail
{ }

Possible aspects for an answer would be performance vs. maintainability vs. readability.

Answer 1

In general, I prefer b) over a) as it explicitly avoids any default construction of members. For ints, doubles etc. that isn't a consideration, but it can be for members with expensive operations or side effects. It's more maintainable if you don't have to consider this potential cost/issue as you're adding and removing members. Initialiser lists also support references and non-default-constructable elements.

Alternatively, you could have a sub-structure for the non-"implementation detail" members and let the compiler generate copying code, along the lines:

struct X 
{
    struct I
    {
        int x_;
        int y_;
    } i_;
    int z_;

    X() { }

    X(const X& rhs)
      : i_(rhs.i_), z_(0) // implementation not copied
    { }

    X& operator=(const X& rhs)
    {
        i_ = rhs.i_;
        return *this;
    } 
};

Answer 2

Normally you implement assignment operator in terms of copy constructor (@Roger Pate's version):

FooBar& operator=(FooBar copy) { swap(*this, copy); return *this; }
friend void swap(FooBar &a, FooBar &b) {/*...*/}

This requires providing a swap function which swaps relevant members (all except implementation_detail in your case).

If swap doesn't throw this approach guarantees that object is not left in some inconsistent state (with only part members assigned).

However in your case since neither copy constructor, nor assignment operator can throw implementing copy constructor in terms of assignment operator (a) is also fine and is more maintainable then having almost identical code in both places (b).

Answer 3

If you're really bothered about replicating std::swap, why not put everything other than the implementation detail into a struct?

struct FooBarCore {
  long id;
  double valX;
  double valZ;
  long   valN;
  bool   flag; 
  // ...
};

struct FooBar {
  FooBarCore core_;
  NonCopyable implementation_detail; // cannot and must not be copied
};

then you can use std::swap for this struct in your copy function for FooBar.

FooBar& operator=(const FooBar &src) {
  FooBarCore temp(src.core_)
  swap(temp,*this.core_);
  return *this;
}

Answer 4

Okay, another try, based on my comment to this answer.

Wrap the implementation_detail in a copyable class:

class ImplementationDetail
{
public:
  ImplementationDetail() {}
  ImplementationDetail(const ImplementationDetail&) {}
  ImplementationDetail& operator=(const ImplementationDetail&) {}

public: // To make the example short
  Uncopyable implementation_detail;
};

and use this class in your FooBar. The default generated Copy Constructor and Copy Assignment Operator for Foobar will work correctly.

Maybe it could even derive from Uncopyable so you don't get implementation_detail.implementation_detail all over your code. Or if you control the code to the implementation_detail class, just add the empty Copy Constructor and empty Assignment Operator.

Answer 5

If the Copy Constructor does not need to copy implementation_detail and still will be correct (I doubt the latter, but let's assume it for the moment), the implementation_detail is superfluous.

So the solution seems to be: make the implementation_detail static and rely on the default generated Copy Constructor and Assignment Operator.