5

Code like this from one of my books for example:

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

HasPtr& HasPtr::operator=(const HasPtr &rhs){
    auto newp = new string(*rhs.ps); // copy the underlying string
    delete ps; // free the old memory
    ps = newp; // copy data from rhs into this object
    i = rhs.i;
    return *this; // return this object
}

Seems like the inside of the operator= could just be:

*ps = *rhs.ps
i = rhs.i
return *this;

With no need to delete the pointer first, seems redundant to do so. It did mention it is written in a way to leave the object in a suitable state should an exception occur but didn't divulge past that, but I don't see what exception could occur that even my alternative wouldn't handle. Why is there a need to delete the object first before assigning?

Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
AntiElephant
  • 1,227
  • 10
  • 18
  • 3
    You need to observe the [rule of three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) (which is trivially observed by following the [rule of zero](http://stackoverflow.com/q/22806523/315052)). Also use [copy-and-swap](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) if you implement your own assignment operator. – jxh Aug 27 '15 at 23:46
  • 1
    @LightnessRacesinOrbit C++ Primer 5th Edition, Chapter 13.2.1 – AntiElephant Aug 28 '15 at 00:04
  • 1
    If you jump to page 644, they provide a fixed version of their assignment operator. – jxh Aug 28 '15 at 00:10

2 Answers2

5

In this case, yes, that would be fine.

You're not leaking the dynamically-allocated string: you're re-using it.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
4

This looks fine to me.

And you're right, std::string assignment already offers a strong exception guarantee so you will still leave the object in its original state should an exception occur copying the string.

Of course there is no reason to allocate a std::string with new like that. You could just write this instead:

class HasNoPtr {
public:
    HasNoPtr(const std::string& s): ps(s), i(0) { }
private:
    std::string ps;
    int i;
};
Chris Drew
  • 14,926
  • 3
  • 34
  • 54