0
\$\begingroup\$

Source/Context:

What is the copy-and-swap idiom?

Rule-of-Three becomes Rule-of-Five with C++11?

The C++ Programming Language 4th Edition June 2013 - Section 17.1 Introduction 483

class X {
X(Sometype);  // ‘‘ordinar y constructor’’: create an object
X();  // default constructor
X(const X&); // copy constructor
X(X&&);  // move constructor
X& operator=(const X&); // copy assignment: clean up target and copy
X& operator=(X&&); // move assignment: clean up target and move
˜X();  // destructor: clean up
// ...
};

There are five situations in which an object is copied or moved: • As the source of an assignment • As an object initializer • As a function argument • As a function return value • As an exception In all cases, the copy or move constructor will be applied (unless it can be optimized away). In addition to the initialization of named objects and objects on the free store, constructors are used to initialize temporary objects (§6.4.2) and to implement explicit type conversion (§11.5). Except for the ‘‘ordinary constructor,’’ these special member functions can be generated by the compiler; see §17.6. This chapter is full of rules and technicalities. Those are necessary for a full understanding, but most people just learn the general rules from examples.

To clarify the concepts in my mind I created a simple X class.

I'm looking for suggestions how my X class could be improved & to check I've not missed something(s)?

I've include a main() for testing; each function prints to std::cout when called.

One thing I don't fully understand: Why use mArray(mSize ? new int[mSize]() : 0) instead of: mArray(mSize) ?

X(std::size_t size = 0)                                                      
                : mSize(size),
                  mArray(mSize ? new int[mSize]() : 0)
            {
                // Default constructor
                // Initialize via constructor
                std::cout << "RETURN: X(std::size_t)" << std::endl;
    }   

#include <iostream>
#include <string>

class X {
    private:
        std::size_t mSize;
        int* mArray;
        void swap(X& other) noexcept
        {
            // Enable ADL (not necessary in our case, but good practice)
            // using std::swap;
            // By swapping the member(s) of two classes,
            // The two classes are effectively swapped
            std::swap(this->mSize, other.mSize);
            std::swap(this->mArray, other.mArray);
        }

    public:
        X(std::size_t size = 0)                                                      
            : mSize(size),
              mArray(mSize ? new int[mSize]() : 0)
        {
            // Default constructor
            // Initialize via constructor
            std::cout << "RETURN: X(std::size_t)" << std::endl;
        }                               
        X(const X& other)                                                                
            : mSize(other.mSize),                                                   
              mArray(mSize ? new int[mSize] : 0)
        {
            // Copy constructor 
            // Initialize via constructor
            // Note: 
            // Non-throwing because of data types being used; more attention to detail with regards
            // to exceptions must be given in a more general case, however
            std::copy(other.mArray, other.mArray + mSize, mArray);
            std::cout << "RETURN: X(const X&)" << std::endl;
        }                   
        X(X&& other)                                                            
            : X()                                                               
        {
            // Move constructor
            // C++11 only: Initialize via Default constructor X() 
            swap(std::move(other));         
            std::cout << "RETURN: X(X&&)" << std::endl;
        }                       

        X& operator=(X& rhs)
            // ?Copy assignment: 
            // Swap LHS with temp RHS - LHS resources released when function returns.
            // Strong exception guarantee. 
            // Self-assignment test not required. 
            // Duplicate code avoided.  
        {               
            X temp(rhs);                
            swap(temp);         
            std::cout << "RETURN: X& operator=(const X&)" << std::endl;
            return *this;
        }       
        X& operator=(X&& rhs)   
            // Move assignment: 
            // Swap LHS with RHS - LHS resources released when function returns.
            // Duplicate code avoided.
        {
            swap(std::move(rhs));                   
            std::cout << "RETURN: X& operator=(X&&)" << std::endl;
            return *this;
        }           
        virtual ~X()                                
        {
            // Destructor: clean up
            delete[] mArray;
            std::cout << "RETURN: virtual ~X()" << std::endl;
        }   

};


int main() {

    std::cout << "X(std::size_t size);  -  Default constructor" << std::endl;
    std::cout << "--------------------------------------------" << std::endl;
    std::cout << "  CALL: X var1(1)" << std::endl;
    X var1(1);
    std::cout << std::endl << std::endl;

    std::cout << "X();  -  Default constructor" << std::endl;
    std::cout << "----------------------------" << std::endl;
    std::cout << "  CALL: X var2 = X()" << std::endl;
    X var2 = X();   
    std::cout << std::endl << std::endl;

    std::cout << "X(const X&);  -  Copy constructor" << std::endl;
    std::cout << "---------------------------------" << std::endl;
    X temp(3);
    std::cout << "  CALL: X var3(temp)" << std::endl;
    X var3(temp);
    std::cout << std::endl << std::endl;

    std::cout << "X(X&&); - Move constructor" << std::endl;
    std::cout << "--------------------------" << std::endl;
    std::cout << "  CALL: X var4( std::move(X(4)) )" << std::endl;
    X var4( std::move(X(4)) );
    std::cout << std::endl << std::endl;

    std::cout << "X& operator=(const X&); - Copy assignment: clean up target and copy" << std::endl;
    std::cout << "-------------------------------------------------------------------" << std::endl;
    X var5 = X(5);
    std::cout << "  CALL: temp = var5;" << std::endl;       
    temp = var5;
    std::cout << std::endl << std::endl;

    std::cout << "X& operator=(X&&); - Move assignment: clean up target and move" << std::endl;
    std::cout << "--------------------------------------------------------------" << std::endl;
    std::cout << "  CALL: var2 = std::move(X(2))" << std::endl;
    var2 = std::move(X(2));
    std::cout << std::endl << std::endl;


    return 0;
}
\$\endgroup\$

closed as off-topic by 200_success Jun 14 '15 at 22:38

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ FWIW, I'm quite critical of the copy-and-swap idiom; a copy-and-move makes more sense IMHO. Not everyone agrees with me, though. \$\endgroup\$ – Veedrac Jun 14 '15 at 22:22
  • 1
    \$\begingroup\$ @Veedrac: Will since a move is usually implemented as a swap (because 1) You can't delete until after the state has been completely changed 2) The src must by in a valid state agter the move) I don't think it makes that much difference. The name is a hangover from pre C++11 when when we did not have move. \$\endgroup\$ – Martin York Jun 14 '15 at 23:06
  • 1
    \$\begingroup\$ @Veedrac: But if you can write an answer to this question What is the copy-and-swap idiom? and explain the improvement achieved by copy and move I will happily vote on it. \$\endgroup\$ – Martin York Jun 14 '15 at 23:12
  • 1
    \$\begingroup\$ @LokiAstari Something like gist.github.com/Veedrac/978c91238ac681323524. It's not much different, but I think it's cleaner. \$\endgroup\$ – Veedrac Jun 15 '15 at 0:44
  • 1
    \$\begingroup\$ @tuk Don't trust VS! Clang and GCC both agree with me here. I had to fix your code a tad to make it compile. // I wasn't sure how to time something so evidently quick, but I threw together some benchmarks compiled with optimizations and got exactly the same time for both of them. The only expensive part is actually the overhead in the loop or allocations when I include them, so you evidently can't perceive a difference. The point being that there's absolutely no way that the time difference you gave is reasonable. \$\endgroup\$ – Veedrac Jun 20 '15 at 22:46
2
\$\begingroup\$

This is not an exhaustive answer.

One thing I don't fully understand: Why use mArray(mSize ? new intmSize : 0) instead of: mArray(mSize) ?

Because you need to initialize the array with a pointer, not with an integer.

X(X&&other)
: X()                                                               
{
    // Move constructor
    // C++11 only: Initialize via Default constructor X() 
    swap(std::move(other));         
    std::cout << "RETURN: X(X&&)" << std::endl;
}

Your constructor calls swap, which receives a reference (meaning you are actually creating a copy here).

Either way, you don't need to write your own swap for this class.

For this constructor, consider this implementation:

X::X(X&& other)
: mSize(other.mSize), mArray(other.mArray)
{
     other.mSize = 0;
     other.mArray = nullptr;
}

Implement a single copy assignment operator, using copy&swap:

X& operator=(X rhs) // pass by value
// Strong exception guarantee. 
{               
    // X temp(rhs); // copy already created in received argument
    using std::swap;
    swap(*this, rhs);

    std::cout << "RETURN: X& operator=(const X&)" << std::endl;
    return *this;
}

Other stuff:

Use nullptr instead of zero.

Either make the class final and the destructor non-virtual, or leave the destructor virtual and remove the assignment operators. Having an assignment operator in a base class leads to getting your objects sliced.

var2 = std::move(X(2));

X(2) is a rvalue; you don't need the move call (just write var2 = X{2};).

\$\endgroup\$
  • \$\begingroup\$ Executing main() { temp = var5 } creates a Stack overflow Exception at: X& operator=(X rhs) { std::swap(*this, rhs) } \$\endgroup\$ – tuk Jun 18 '15 at 14:04

Not the answer you're looking for? Browse other questions tagged or ask your own question.