Является ли это правильной реализацией правила пяти (или правила четырех и 1/2)?

Я изучаю правило пяти и его кузенов (правило четырех и 1/2, идиома копирования и обмена, функция обмена друзьями).

Я применил Правило четырех и 1/2 на тестовом классе. Он хорошо компилируется. Есть ли скрытая ошибка в моей реализации?

Меня особенно беспокоят уникальные_ptr, хранящиеся в свойстве m_unorederd_map , которые я переместил в конструкторе копирования, поскольку их нельзя скопировать. Это правильный способ работы с unique_ptrs в классах?

какой-то класс.h

      #ifndef SOMECLASS_H
#define SOMECLASS_H

#include "someotherclass.h"

#include <QString>
#include <QStringList>

#include <memory>
#include <string>
#include <unordered_map>

class SomeClass
{
    QString m_qstring;
    std::unordered_map<std::string, std::unique_ptr<SomeOtherClass>> m_unordered_map;
    int m_int;
    std::string m_string;
    QStringList m_qstringlist;

public:
    SomeClass() = default;

    // Rule of 5 (or Rule of 4 and 1/2)
    // From https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom#3279550
    ~SomeClass() = default;                               // Destructor
    SomeClass(SomeClass &other);                          // Copy constructor
    SomeClass(SomeClass &&other);                         // Move constructor
    SomeClass &operator=(SomeClass other);                // Copy/Move assignment operator
    friend void swap(SomeClass &first, SomeClass &second) // Friend swap function
    {
        using std::swap;
        first.m_qstring.swap(second.m_qstring);
        first.m_unordered_map.swap(second.m_unordered_map);
        swap(first.m_int, second.m_int);
        swap(first.m_string, second.m_string);
        first.m_qstringlist.swap(second.m_qstringlist);
    }
};

#endif // SOMECLASS_H

какой-то класс.cpp

      #include "someclass.h"

// Copy constructor
SomeClass::SomeClass(SomeClass &other)
    : m_qstring(other.m_qstring),
      m_int(other.m_int),
      m_string(other.m_string),
      m_qstringlist(other.m_qstringlist)
{
    // m_unordered_map holds unique_ptrs which can't be copied.
    // So we move it.
    m_unordered_map = std::move(other.m_unordered_map);
}

// Move constructor
SomeClass::SomeClass(SomeClass &&other)
    : SomeClass()
{
    swap(*this, other);
}

// Copy/Move assignment operator
SomeClass &SomeClass::operator=(SomeClass other)
{
    swap(*this, other);
    return *this;
}

1 ответ

Самое важное из всех:

  • Этот класс не нуждается ни в пользовательских операциях копирования/перемещения, ни в деструкторе, поэтому следует следовать правилу 0.

Другие вещи:

  • мне не нравится swap(*this, other);в движении ctor. Это заставляет элементы создаваться по умолчанию, а затем назначаться. Лучшей альтернативой было бы использование списка инициализаторов элементов с std::exchange.

    Если инициализация всех членов становится утомительной, оберните их в структуру. Это заставляет писать swapтоже легче.

  • Конструктор копирования должен принимать параметр по константной ссылке.

  • unique_ptrs which can't be copied. So we move it.является плохим обоснованием. Если ваши члены не могут быть скопированы, не определяйте операции копирования. При наличии пользовательских операций перемещения операции копирования не будут генерироваться автоматически.

  • Операции перемещения (включая присвоение по значению) должны быть noexcept, так как стандартные контейнеры не будут использовать их иначе в некоторых сценариях.

  • SomeClass() = default;вызывает члены, которые обычно не инициализированы ( int m_int;), чтобы иногда обнуляться, в зависимости от того, как построен класс. (Например SomeClass x{};обнуляет, но SomeClass x;нет.)

    Если вы не хотите такого поведения, конструктор следует заменить на SomeClass() {}, а также m_intвероятно, следует обнулить (в теле класса).

Другие вопросы по тегам