Your code has a problem with self-assignment and with exceptions: assume that the memory allocation throws a std::bad_alloc
exception. When coding, you should always assume that memory allocations can go wrong although the actually rarely do. In the code
delete ps;
ps = new std::string(*hp.ps);
ps
would point to stale member when the second line of code throws an exception. Incidentally, if you end up self-assigning the object, you actually delete
the memory of the only before accessing it. Thus, it is a good idea to first copy the content of the right hand side, then put things into place, and finally release resource.
As it happens, these are exactly the operations of
- the copy constructor
- a
swap()
operation you generally want for any type holding resources - the destructor
The way to leverage these three operation is known as copy and swap idiom:
T& T::operator=(T other) {
this->swap(other);
return *this;
}