Copy assignment definition for a user-defined type

[frek]

Good day guys,

Is the implementation for the assignment operator below correct, please? If not, why?

class Vector {

 private:
	double* elem; // elem points to an array of sz doubles
	int sz;

 public:
	Vector(int s); // constructor: establish invariant, acquire resources
	~Vector() { delete[] elem; } // destructor: release resources
	Vector(const Vector& a); // copy constructor
	Vector& operator=(const Vector& a); // copy assignment
	double& operator[](int i);
	const double& operator[](int i) const;

	int size() const;
};

Vector& Vector::operator=(const Vector& rhs) // copy assignment
{
	delete[] elem; // delete old elements
	elem = new double[rhs.sz];
	for (int i = 0; i != rhs.sz; ++i)
		elem[i] = rhs.elem[i];
	sz = rhs.sz;
	return *this;
}

[seeplus]

No. What happens if you try to assign to itself?

[mbozzi]

For some Vector v, the expression v = v immediately deletes v.elem. So a check for self-assignment is required.

Vector& Vector::operator=(const Vector& rhs) // copy assignment
{
        if (this != &rhs)
 	{
                delete[] elem; // delete old elements
	        elem = new double[rhs.sz];
	        for (int i = 0; i != rhs.sz; ++i)
		        elem[i] = rhs.elem[i];
	        sz = rhs.sz;
        }
	return *this;
}

Further:

Copy construction should typically be exception-safe. However, if an exception is thrown from line line 21, data loss will result. Consider allocating memory & copying first so that no user data will be lost if this process throws an exception (fails).

In general, assignment of user-defined types can fail too - but we're okay here because double never throws exceptions when assigned.

Vector& Vector::operator=(const Vector& rhs) // copy assignment
{
        if (this != &rhs)
 	{
                auto new_elem = new double[rhs.sz];
	        for (int i = 0; i != rhs.sz; ++i)
		        new_elem[i] = rhs.elem[i];
                delete[] elem; // delete old elements
                elem = new_elem;
	        sz = rhs.sz;
        }
	return *this;
}

The situation becomes significantly more complicated once Vector becomes a class template.

[JLBorges]

class Vector {

 private:
	double* elem; // elem points to an array of sz doubles
	int sz;

 public:
	Vector(int s); // constructor: establish invariant, acquire resources
	~Vector() { delete[] elem; } // destructor: release resources
	Vector(const Vector& a); // copy constructor
	Vector& operator=(const Vector& a); // copy assignment
	double& operator[](int i);
	const double& operator[](int i) const;

	int size() const;

	Vector& assign_in_place( const Vector& that ) noexcept ; // invariant: size() >= that.size()
};

Vector& Vector::operator= ( const Vector& that ) {

    if( this != std::addressof(that) )
    {
        // reallocate only if existing storage is insufficient
        if( size() >= that.size() ) return assign_in_place(that) ;

        // we do need reallocation, a (slightly unusual, perhaps) deferred copy and swap
        Vector temp = that ; // copy construct, if this throws, we are safe

        // this won't throw.
        using std::swap ;
        swap( elem, temp.elem ) ;
        swap( sz, temp.sz ) ;
    }

    return *this ;
}

[frek]

@JLBorges

1) On line 17, assign_in_place does an std::copy, right? By noexcept we tell the function that no exception should be thrown, right? For what functions/ctos etc do we need to use that keyword, please?

2) On line 28 both temp and that have the same address and contents, right?

3) On line 32, if std::swap, swaps the contents of its parameters and won't throw, then why did we need a temp vector (line 28)?

[JLBorges]

> By noexcept we tell the function that no exception should be thrown, right?

We tell the caller of the function (and the compiler) that we do not expect this function to propagate an exception to its caller.


> For what functions/ctos etc do we need to use that keyword, please?

Move constructors and move assignment operators should almost always be having a non-throwing guarantee (Destructors too, but they are noexcept by default). For other functions, this is the rule that I follow: "If it is clear that the current and future implementations of this function should not throw, then I specify it as noexcept"


> On line 28 both temp and that have the same address and contents, right?

They have the same contents; temp is a copy of that. These are two different objects; they have different addresses.


> On line 32, if std::swap, swaps the contents of its parameters and won't throw, then why did we need a temp vector (line 28)?

The copy constructor of the temp Vector is the one that acquires the the resources needed for making a copy. If the acquisition of new resources by temp is successful, the swap logically exchanges the contents of our vector and temp; we now hold a copy of the Vector that and our old contents would be released by the destructor of the Vector temp.

More information: https://en.wikibooks.org/wiki/More_C++_Idioms/Copy-and-swap

[frek]

I mean if we defined the operator the following way what would be the problem (while std::swap doesn't throw and we haven't the fear of losing data by it)?

Vector& Vector::operator= ( const Vector& that ) {

    if( this != std::addressof(that) )
    {
        // reallocate only if existing storage is insufficient
        if( size() >= that.size() ) return assign_in_place(that) ;

        std::swap( elem, that.elem ) ;
        std::swap( sz, that.sz ) ;
    }

    return *this ;
}

[JLBorges]

> if we defined the operator the following way what would be the problem

A compile time error.