Это хороший код? (оператор копирования и оператор присваивания)

По той или иной причине я вынужден предоставить как конструктор копирования, так и оператор = для моего класса. Я думал, что мне не нужно operator=, если я определил копию ctor, но QList хочет ее. Отложив это в сторону, я ненавижу дублирование кода, так что есть что-то не так с этим?

Fixture::Fixture(const Fixture& f) {
    *this = f;
}

Fixture& Fixture::operator=(const Fixture& f) {
    m_shape         = f.m_shape;
    m_friction      = f.m_friction;
    m_restitution   = f.m_restitution;
    m_density       = f.m_density;
    m_isSensor      = f.m_isSensor;
    return *this;
}

И просто из любопытства, нет способа переключить его так, чтобы основная часть кода была в копии ctor и operator= каким-то образом ее использует? Я пробовал return Fixture(f);, но мне это не понравилось.

Мне кажется, мне нужно сделать более понятным, что конструктор копирования и оператор присваивания были неявно отключены классом, который я наследую. Зачем? Потому что это абстрактный базовый класс, который не следует создавать самостоятельно. Этот класс, однако, предназначен для самостоятельной работы.

Ответ 1

Это плохо, потому что operator= больше не может полагаться на объект настройки. Вы должны сделать это наоборот, и можете использовать идиому копирования.

В случае, когда вам просто нужно скопировать все элементы, вы можете использовать неявно сгенерированный оператор присваивания.

В других случаях вам придется делать что-то дополнительно, в основном освобождая и копируя память. Именно здесь подходит идиома с копией. Он не только элегантен, но и обеспечивает так, что задание не генерирует исключений, если оно только свопирует примитивы. Пусть класс указывает на буфер, который вам нужно скопировать:

Fixture::Fixture():m_data(), m_size() { }

Fixture::Fixture(const Fixture& f) {
    m_data = new item[f.size()];
    m_size = f.size();
    std::copy(f.data(), f.data() + f.size(), m_data);
}

Fixture::~Fixture() { delete[] m_data; }

// note: the parameter is already the copy we would
// need to create anyway. 
Fixture& Fixture::operator=(Fixture f) {
    this->swap(f);
    return *this;
}

// efficient swap - exchanging pointers. 
void Fixture::swap(Fixture &f) {
    using std::swap;
    swap(m_data, f.m_data);
    swap(m_size, f.m_size);
}

// keep this in Fixture namespace. Code doing swap(a, b)
// on two Fixtures will end up calling it. 
void swap(Fixture &a, Fixture &b) {
  a.swap(b);
}

Как обычно я пишу оператор присваивания. Прочитайте Хотите скорость? Передайте по значению о необычной сигнатуре оператора присваивания (перейдите по значению).

Ответ 2

Копирование ctor и присваивание полностью различны - для назначения обычно требуется освобождать ресурсы в объекте, который он заменяет, copy ctor работает над еще не инициализированным объектом. Поскольку здесь у вас, по-видимому, нет особых требований ( "освобождение" не требуется при назначении), ваш подход прекрасен. В общем, у вас может быть "свободный все ресурсы, которые объект держит", вспомогательный метод (который вызывается в dtor и в начале назначения), а также часть "копировать эти другие вещи в объект", которые достаточно близки к работе типовой копии ctor (или, в основном, в любом случае; -).

Ответ 3

Вы просто делаете копию и назначение членов в своих примерах. Это не то, что вам нужно написать. Компилятор может генерировать неявные операции копирования и присваивания, которые выполняют именно это. Вам нужно только написать свой собственный конструктор, назначение и/или деструктор, если создаваемые компилятором не подходят (например, если вы управляете каким-то ресурсом через указатель или что-то в этом роде)

Ответ 4

Я думаю, что у вас возникают проблемы, если ваш оператор = когда-либо становится виртуальным.

Я бы рекомендовал написать функцию (может быть, статическую), которая делает копию, тогда вместо этого будет создан экземпляр-копир, а оператор = вызывает эту функцию.

Ответ 5

Да, это хорошая практика и должна (почти) всегда выполняться. Кроме того, бросать в деструктор и конструктор по умолчанию (даже если вы делаете его закрытым).

В книге Джеймса Коплиена 1991 года Advanced С++, это описано как часть "Православной канонической формы". В нем он выступает за конструктор по умолчанию, конструктор копирования, оператор присваивания и деструктор.

В общем, вы должны использовать ортодоксальную каноническую форму, если:

Вы хотите поддерживать назначение объекта класса или хотите передать эти объекты в качестве параметров по умолчанию для функции и

Объект содержит указатели на объекты, которые подсчитываются по ссылке, или деструктор класса выполняет delete в элементе данных объекта.
Вы должны использовать ортодоксальную каноническую форму для любого нетривиального класса в программе ради равномерности между классами и управлять возрастающей сложностью каждого класса в ходе эволюции программы.

Coplien предлагает страницы причин для этого шаблона, и я не мог сделать их справедливыми здесь. Тем не менее, ключевым элементом, который уже был затронут, является возможность очистки объекта, который перезаписывается.

Ответ 6

Я думаю, что вы должны инициализировать переменные-члены объекта, используя initializer list. Если ваши переменные имеют значение primitive-types, то это не имеет значения. В противном случае назначение отличается от инициализации.

Вы можете сделать это с помощью небольшого трюка, инициализируя указатели внутри copy constructor до 0, затем вы можете безопасно вызвать удаление в assignment operator:

Fixture::Fixture(const Fixture& f) : myptr(0) {
    *this = f;
}
Fixture& Fixture::operator=(const Fixture& f) {
    // if you have a dynamic array for example, delete other wise.
    delete[] myptr;
    myptr = new int[10];
    // initialize your array from the other object here.
    ......
    return *this;
}