Implementing valuelike copy-assignment operator

Question

In C++ Primer there is an example of using the copy-control members to make a class act "valuelike"; that is, when copying the object, the copies are independent. It proposes the following code:

class HasPtrValue
{
public:
    HasPtrValue(const std::string &s = std::string()) : ps(new std::string(s)), i(0) {  }
    HasPtrValue(const HasPtrValue &orig) : ps(new std::string(*orig.ps)), i(orig.i) { }
    HasPtrValue& operator=(const HasPtrValue&);
    ~HasPtrValue() { delete ps; };

    std::string *ps;
    int i;
};

HasPtrValue& HasPtrValue::operator=(const HasPtrValue &rhs)
{
    auto newp = new std::string(*rhs.ps);
    delete ps;
    ps = newp;
    i = rhs.i;
    return *this;
}

My question is regarding the copy-assignment operator. As I understand what it does is create a new string on the heap, delete the old one, and make the lhs point to the new one. Is that really necessary? Will the below code not do exactly the same thing by simply assigning to the existing string on the heap?

HasPtrValue& HasPtrValue::operator=(const HasPtrValue &rhs)
{
    *ps = *rhs.ps;
    i = rhs.i;
    return *this;
}

The entire class is useless, since it could store `std::string` directly and have it managed automagically. — milleniumbug, Feb 24 '15 at 20:00
Your observation is correct, though the closing wording a little odd. Apart from the uselessness of the class as a whole, I realize it is an academic exercise. The change you propose will invoke the copy-assignment operator of `std::string`, and direct assignment of the `int` member `i`. The former does indeed internally live on the heap. The choice of code for *their* assignment operator is honestly not the way most people would do this regardless, as it is anything-but exception-friendly, and in fact, *none* of this is how one would normally do this at all. — WhozCraig, Feb 24 '15 at 20:03
Sure, I wouldn't use this in an actual program. It's just for proof of concept. @milleniumbug — JamesLens, Feb 24 '15 at 20:04
What happens to the memory that was pointed to by ps if you just reassigned it? — NathanOliver, Feb 24 '15 at 20:04
@NathanOliver if you mean if you simply did `ps = rhs.ps;` that would be a recipe for disaster. — WhozCraig, Feb 24 '15 at 20:05
@NathanOliver The pointer still points to the same memory location in my code, just the string it points to is changed. — JamesLens, Feb 24 '15 at 20:06
@WhozCraig Thanks, that makes sense. Their version does look quite strange. — JamesLens, Feb 24 '15 at 20:08
@NathanOliver `*ps` and `*rhs.ps` are standard library `std::string` objects. Don't confuse the pointer `ps` with what it *points to*. The dereferences are important. — WhozCraig, Feb 24 '15 at 20:10
@WhozCraig: I missed the dereference on the pointers. I thought I saw `ps = rhs.ps;` not `*ps = *rhs.ps;` — NathanOliver, Feb 24 '15 at 20:10
@WhozCraig at least they `new` before `delete` so that if the new throws there isn't a leak; and destructors aren't supposed to throw — M.M, Feb 24 '15 at 20:22
@NathanOliver I did the same and got downvoted into oblivion. Funny how missing a single * character in such a contrived example can make all the difference. — Julian, Feb 24 '15 at 20:47

Vlad from Moscow · Answer 1 · 2015-02-24T20:30:29.527

You are correct. It will be enough to define the copy assignment operator the following way

HasPtrValue& HasPtrValue::operator=(const HasPtrValue &rhs)
{
    *ps = *rhs.ps;
    i = rhs.i;
    return *this;
}

The only difference (apart from allocation a new memory) is that in this case the string can contain much reserved memory though the string of rhs object can be enough small.

The C++ Standard does not say that the destination string will be shrink to the size of the original string when the copy assignment operator is used. It only says that

size() str.size()
capacity() a value at least as large as size()

As for the original version then it should check whether there is self-assignment that to avoid a redundant memory allocation. That is it should look like

HasPtrValue& HasPtrValue::operator=(const HasPtrValue &rhs)
{
    if ( this != &rhs )
    {
        auto newp = new std::string(*rhs.ps);
        delete ps;
        ps = newp;
        i = rhs.i;
    }

    return *this;
}

Checking for self-assignment looks like a pessimisation to me. — mfuchs, Feb 24 '15 at 20:37

score 2 · Accepted Answer · edited May 23 '17 at 11:50

2

You are correct. Your version does not only work it is also more efficient since existing memory can be reused when ps->capacity() >= rhs.ps->capacity().

If you want to provide strong exception guarantees you should use the copy-and-swap idiom:

HasPtrValue& HasPtrValue::operator=(HasPtrValue copy) // notice the by value
{
    swap(*this, copy);
    return *this;
}

// integrating tip from link posted by WhozCraig 
friend void swap(HasPtrValue &lhs, HasPtrValue &rhs)
{
    using std::swap;
    swap(lhs.ps, rhs.ps);
    swap(lhs.i, rhs.i);
}

Though your changes to the code should already provide the strong exception guarantee, as long as the assignment of i is not reordered by the compiler.

edited May 23 '17 at 11:50

Community

1
1

answered Feb 24 '15 at 20:13

mfuchs

2,190
12
20

For the OP's benefit, outstanding question and answers concerning the idea behind the [copy-swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). Worth noting this does essentially what the first code does, but considerably safer. – WhozCraig Feb 24 '15 at 20:14
Both you and Vlad raise a good point, I didn't consider what happens when ps->capacity() >= rhs.ps->capacity(). Indeed, the memory is reused, but what happens when there isn't enough contiguous memory on the heap to accommodate the new string length? If the string is relocated to another place, won't the pointer not be updated? Thanks for the copy-swap idiom, I'll read up. – JamesLens Feb 24 '15 at 20:25
`std::string` does all that internally. It has amongst others a pointer to the actual string data. If there is not enough capacity left new memory will be allocated and if successful the old will be deleted. But as mentioned that is all `std::string` internal so you don't have to bother about it. This is also why you should always prefer `std::string` over c-strings. – mfuchs Feb 24 '15 at 20:28

Implementing valuelike copy-assignment operator

2 Answers2