I was recently implementing the copy constructor and copy assignment operator for a class that manages a dynamically allocated object, and learnt about the copy-and-swap idiom (thanks to this great answer). I ended up with the following implementation, which I believe is correct and satisfies me well :
class MyArray {
int* arr;
int size;
public:
...
~MyArray() { delete[] arr; }
// copy constructor
MyArray(const MyArray& other) {
size = other.size;
arr = new int[size];
for (int i = 0; i < size; ++i) {
arr[i] = other.arr[i];
}
}
friend void swap(MyArray& a1, MyArray& a2);
// copy assignment operator, using copy-and-swap idiom
MyArray& operator=(const MyArray& other) {
MyArray tmp{other};
swap(tmp, *this);
return *this;
}
};
void swap(MyArray& a1, MyArray& a2) {
std::swap(a1.size, a2.size);
std::swap(a1.arr, a2.arr);
}
This works well and is pretty clean (I know using smart pointers would be even better, but it is not the topic of this question).
Still, I am wondering why it is rarely mentioned that you can just implement the copy assignment operator, and then call it inside the copy constructor, like so :
class MyArray {
int* arr;
int size;
public:
...
~MyArray() { delete[] arr; }
// copy assignment operator
MyArray& operator=(const MyArray& other) {
if (this != &other) {
delete[] arr;
size = other.size;
arr = new int[size];
for (int i = 0; i < size; ++i) {
arr[i] = other.arr[i];
}
}
return *this;
}
// copy constructor just calling the copy assignment operator
MyArray(const MyArray& other) : arr(nullptr) { *this = other; }
};
The main pitfall of this approach I can think of is if you forget to assign arr
to nullptr
in the copy constructor before the call to the copy assignment operator, you end up with a segmentation fault when it tries to delete[] arr
.
Still, are there other reasons this would be considered bad practice ? The copy-and-swap idiom is definitely more verbose, so why is it always preferred ?