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

Question

По той или иной причине я вынужден предоставить для своего класса и конструктор копирования, и оператор =.Я думал, мне не нужно operator= если бы я определил копировщик, но 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;
}

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

---

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

Answer 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's namespace. Code doing swap(a, b)
// on two Fixtures will end up calling it. 
void swap(Fixture &a, Fixture &b) {
  a.swap(b);
}

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

Answer 2

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

Answer 3

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

Answer 4

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

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

Answer 5

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

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

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

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

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

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

Answer 6

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

---

Вы можете сделать это с помощью небольшого трюка, инициализируя указатели внутри copy constructor к 0, то вы можете безопасно вызвать delete в 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;
}