Copy constructor and assignment operator implementation choices -

Question

I recently revisited the copy constructor, assignment operator, copy swap idom seen here: What is the copy-and-swap idiom? and many other places -

The Above link is an excellent post - but I still had a few more questions - These questions are answered in a bunch of places, on stackoverflow and many other sites, but I have not seen a lot of consistency -

1 - Should you have try-catch around the areas where we allocate the new memory for a deep copy in the copy constructor ? (Ive seen it both ways)

2 - With regards to inheritance for both the copy constructor and assignment operator, when should the base class functions be called, and when should these functions be virtual?

3 - Is std::copy the best way for duplicating memory in the copy constructor? I have seen it with memcpy, and seen others say memcpy the worst thing on earth.

Consider the example Below (Thanks for all the feedback), it prompted some additional questions:

4 - Should we be checking for self assignment? If so where

5 - Off topic question, but I have seen swapped used as : std::copy(Other.Data,Other.Data + size,Data); should it be: std::copy(Other.Data,Other.Data + (size-1),Data); if swap goes from 'First to Last' and the 0th element is Other.Data?

6 - Why doesn't the commented out constructor work (I had to change size to mysize) - is assume this means regardless of the order I write them, the constructor will always call the allocation element first?

7 - Any other comments on my implementation? I know the code is useless but i'm just trying to illustrate a point.

class TBar
{

    public:

    //Swap Function        
    void swap(TBar &One, TBar &Two)
    {
            std::swap(One.b,Two.b);
            std::swap(One.a,Two.a);
    }

    int a;
    int *b;


    TBar& operator=(TBar Other)
    {
            swap(Other,*this);
            return (*this);
    }

    TBar() : a(0), b(new int) {}                //We Always Allocate the int 

    TBar(TBar const &Other) : a(Other.a), b(new int) 
    {
            std::copy(Other.b,Other.b,b);
            *b = 22;                                                //Just to have something
    }

    virtual ~TBar() { delete b;}
};

class TSuperFoo : public TBar
{
    public:

    int* Data;
    int size;

    //Swap Function for copy swap 
    void swap (TSuperFoo &One, TSuperFoo &Two)
    {
            std::swap(static_cast<TBar&>(One),static_cast<TBar&>(Two));
            std::swap(One.Data,Two.Data);
            std::swap(One.size,Two.size);
    }

    //Default Constructor
    TSuperFoo(int mysize = 5) : TBar(), size(mysize), Data(new int[mysize]) {}
    //TSuperFoo(int mysize = 5) : TBar(), size(mysize), Data(new int[size]) {}                *1

    //Copy Constructor
    TSuperFoo(TSuperFoo const &Other) : TBar(Other), size(Other.size), Data(new int[Other.size])        // I need [Other.size]! not sizw
    {
            std::copy(Other.Data,Other.Data + size,Data);        // Should this be (size-1) if std::copy is First -> Last? *2
    }

    //Assignment Operator
    TSuperFoo& operator=(TSuperFoo Other)
    {
            swap(Other,(*this));
            return (*this);
    }

    ~TSuperFoo() { delete[] Data;}

};

Anthony Williams · Accepted Answer · 2012-06-27T07:38:47.587

If you allocate memory then you need to ensure that it is freed in the case of an exception being thrown. You can do this with an explicit try/catch, or you can use a smart pointer such as std::unique_ptr to hold the memory, which will then be automatically deleted when the smart pointer is destroyed by stack unwinding.
You very rarely need a virtual assignment operator. Call the base class copy constructor in the member initialization list, and base-class assignment operator first in the derived assignment operator if you are doing a memberwise assignment --- if you are doing copy/swap then you don't need to call the base class assignment in your derived assignment operator, provided that copy and swap are implemented correctly.
std::copy works with objects, and will correctly call copy constructors. If you have plain POD objects then memcpy will work just as well. I'd go for std::copy in most cases though --- it should be optimized to memcpy under the hood anyway for PODs, and it avoids the potential for errors should you add a copy constructor later.

[Updates for updated question]

With copy/swap as written there is no need to check for self-assignment, and indeed no way of doing so --- by the time you enter the assignment operator other is a copy, and you have no way of knowing what the source object was. This just means that self-assignment will still do a copy/swap.
std::copy takes a pair of iterators (first, first+size) as input. This allows for empty ranges, and is the same as every range-based algorithm in the standard library.
The commented out constructor doesn't work because the members are initialized in the order they are declared, regardless of the order in the member initializer list. Consequently, Data is always initialized first. If the initialization depends on size then it will get a duff value since size hasn't been initialized yet. If you swap the declarations of size and data then this constructor will work fine. Good compilers will warn about the order of member initialization not matching the order of declarations.

Thanks - Updated with example – MikeyG Jun 26 '12 at 16:23 — MikeyG, Jun 26 '12 at 16:23

score 3 · Answer 2 · answered Jun 26 '12 at 14:09

1 - Should you have try-catch around the areas where we allocate the new memory for a deep copy in the copy constructor ?

In general, you should only catch an exception if you can handle it. If you have a way of dealing with an out-of-memory condition locally, then catch it; otherwise, let it go.

You should certainly not return normally from a constructor if construction has failed - that would leave the caller with an invalid object, and no way to know that it's invalid.

2 - With regards to inheritance for both the copy constructor and assignment operator, when should the base class functions be called, and when should these functions be virtual?

A constructor can't be virtual, since virtual functions can only be dispatched by an object, and there is no object before you create it. Usually, you wouldn't make assignment operators virtual either; copyable and assignable classes are usually treated as non-polymorphic "value" types.

Usually, you'd call the base class copy constructor from the initialiser list:

Derived(Derived const & other) : Base(other), <derived members> {}

and if you're using the copy-and-swap idiom, then your assignment operator wouldn't need to worry about the base class; that would be handled by the swap:

void swap(Derived & a, Derived & b) {
    using namespace std;
    swap(static_cast<Base&>(a), static_cast<Base&>(b));
    // and swap the derived class members too
}
Derived & Derived::operator=(Derived other) {
    swap(*this, other);
    return *this;
}

3 - Is std::copy the best way for duplicating memory in the copy constructor? I have seen it with memcopy, and seen others say memcopy the worst thing on earth.

It's rather unusual to be dealing with raw memory; usually your class contains objects, and often objects can't be correctly copied by simply copying their memory. You copy objects using their copy constructors or assignment operators, and std::copy will use the assignment operator to copy an array of objects (or, more generally, a sequence of objects).

If you really want, you could use memcpy to copy POD (plain old data) objects and arrays; but std::copy is less error-prone (since you don't need to provide the object size), less fragile (since it won't break if you change the objects to be non-POD) and potentially faster (since the object size and alignment are known at compile time).

Thanks - I updated the question with an example and some further discussion, Please take a look — MikeyG, Jun 26 '12 at 19:01
@MikeyG: That's rather a lot of questions; you'd be better off asking them separately. Briefly: (4) you don't need to handle self-assignment if you're using copy-and-swap; (5) `std::copy` (and in general functions taking iterator ranges) expect the "end" iterator to be *past* the end of the sequence; (6) members are always initialised in the order they're declared in the class definition, so `Data` is initialised before `size`. — Mike Seymour, Jun 26 '12 at 19:28

score 1 · Answer 3 · answered Jun 26 '12 at 14:01

If the constructor for what you're deep copying may throw something you can handle, go ahead and catch it. I'd just let memory allocation exceptions propagate, though.
Copy constructors (or any constructors) can't be virtual. Include a base class initializer for these. Copy assignment operators should delegate to the base class even if they're virtual.
memcpy() is too low-level for copying class types in C++ and can lead to undefined behavior. I think std::copy is usually a better choice.

score 1 · Answer 4 · answered Jun 26 '12 at 14:05

try-catch can be used when you have to undo something. Otherwise, just let the bad_alloc propagate to the caller.
Calling the base class' copy constructor or assignment operator is the standard way of letting is handle its copying. I have never seen a use case for a virtual assignment operator, so I guess they are rare.
std::copy has the advantage that it copies class objects correctly. memcpy is rather limited on what types it can handle.

Copy constructor and assignment operator implementation choices -

4 Answers4