Должен ли я использовать новое размещение в операторе присваивания копии

Я создаю дискриминируемый класс, подобный союзу. Я использую С++ 17, поэтому технически я мог бы использовать std::variant, но из-за конкретного варианта использования я хочу, чтобы значение каждого варианта было более явным (особенно потому, что два случая не содержат данных, кроме того, какой случай они есть). Класс выглядит примерно так (для простоты я буду игнорировать семантику перемещения в вопросе):

class MyC {

  public:
  enum class Kind {A, B, C, D};

  private:
  Kind _kind;
  union {

    struct {} _noVal;
    string _aVal;
    int _bVal; 

  };

  MyC(Kind kind) : _kind(kind), _noVal() {}

  public:
  MyC(const MyC& other) : _kind(other.kind), _noVal() {
    if (_kind == Kind::A) new (&_aVal) string(other._aVal);
    if (_kind == Kind::B) _bVal = other._bVal;
  }

  ~MyC() {
    if (_kind == Kind::A) _aVal.~string();
  }

  MyC& operator =(const MyC&);

  // factory methods and methods for consuming the current value

}

Моя первая мысль об операторе присваивания копии:

MyC& MyC::operator &(const MyC& other) {
  this->~MyC();
  _kind = other._kind;
  if (_kind == Kind::A) new (&_aVal) string(other.aVal);
  else if (_kind == Kind::B) _bVal = other.bVal;
  else _noVal = other.noVal;
  return *this;
}

Мне это кажется прекрасным, но мне интересно, лучше ли стиль С++ вызывать оператор присваивания строки, для которого потребуется что-то вроде этого:

MyC& MyC::operator &(const MyC& other) {
  if (other._kind == Kind::A) {
    if (_kind != Kind::A) new (&_aVal) string; // *
    _aVal = other.aVal;
  } else if (other._kind == Kind::B) {
    _bVal = other.bVal;
  } else {
    _noVal = other.noVal;
  }
  _kind = other._kind;
  return *this;
}

Подводя итог, как правильно это сделать (и почему) или это имеет значение?

* Эта строка здесь, потому что моя первоначальная реализация установила aVal напрямую, не убедившись, что там когда-либо была инициализирована строка, и это привело к сбою.


c++ copy-assignment placement-new
person Anonymous    schedule 06.11.2019    source источник
comment
Как насчет использования существующего протестированного кода и сохранения std::variant в вашем классе вместо union? Вы можете сохранить дополнительное поведение/требования, просто реализуя меньше кода.   -  person Richard Critten    schedule 06.11.2019
comment
Я мог бы, но у меня сложилось впечатление, что std::variant имеет больше накладных расходов, и, поскольку мой класс действительно довольно прост, это не большая проблема, если только нет какой-то действительно большой и сложной проблемы, которую я действительно не хочу исправлять самостоятельно ( как с std::shared_ptr).   -  person Anonymous    schedule 06.11.2019
comment
Я настоятельно рекомендую вам использовать std::variant; единственные накладные расходы памяти, которые у него есть, - это один size_t (я полагаю) для индекса типа, что не так много. Эти низкоуровневые классы, такие как std::variant, могут быть очень сложными для правильной работы, особенно если вы хотите иметь надлежащую безопасность исключений.   -  person HolyBlackCat    schedule 07.11.2019

Ответы (1)


arrow_upward
0
arrow_downward

Ваша первая версия заставляет меня нервничать, потому что вы никогда не вызываете конструктор для this после вызова this->~MyC(), поэтому ваш объект следует считать недействительным. Даже если код сразу после принудительно приводит все текущие переменные-члены к теоретической достоверности, он будет подвержен ошибкам.

Ваш второй вариант кажется лучше, но он по-прежнему оставляет три места, которые должны обновляться каждый раз, когда кто-то добавляет в этот союз... и учитывая, что ваше перечисление Kind имеет 4 значения, а вы показываете только три... Я думаю, что произойдет рано или поздно. Опять же, склонен к ошибкам.

Я могу придумать еще два варианта:

1) 

MyC& operator =(const MyC& other)
{
    this->~MyC();
    new (this) MyC(other);
}

Используйте существующий конструктор копирования на месте после использования деструктора «на месте», чтобы избежать утечек. Слегка хакерский, намного чище, без дополнительного обслуживания.

Это приводит к тому, что у вас есть только два места, где вам нужно внести изменения, за счет небольшой эффективности в случае строки в строку. Я считаю, что ремонтопригодность превосходит эффективность в большинстве случаев. Какие случаи? Спросите у своего профилировщика.

ПРЕДУПРЕЖДЕНИЕ. Как указал Крис в комментариях ниже, это приведет к ужасной ошибке при работе с классом, производным от MyC, если этот класс также не имеет собственного оператора присваивания. Если бы он был вызван в каком-то производном классе, этот класс

  • утечка всего, что требовало вызова деструктора производного класса.
  • turn the derived class into an instance of MyC. MOSTLY. I suspect that the derived destructor would still be called, with much potential badness to follow.
    • OTOH, this could just clean up all the stuff that 'leaked' when we called the wrong destructor earlier. It really Just Depends.

С одной стороны, этот код — отличный пример ходьбы по высокой веревке без сетки. С другой стороны, так работает с new/delete напрямую. Просто у нас больше практики хождения по этой конкретной веревке, и у нас есть такие сети, как unique_ptr и shared_ptr, которые мы можем (и, как правило, должны) использовать.

2)

Оберните свой союз в какой-нибудь шаблон any/variant (std::variant, boost::any, что угодно), согласно комментарию Ричарда. У моего последнего работодателя было что-то вроде 3 разных вариантов/любых классов, написанных тремя разными программистами, по крайней мере двое из которых явно не удосужились сначала просмотреть нашу кодовую базу. У вашей компании тоже может быть своя реализация.

Не думайте, что они будут менее эффективными. Опять же: спросите своего профилировщика.

несвязанный: поиск в кодовой базе бывшего работодателя интересных фраз, таких как «не регистрироваться», ненормативной лексики и т. д., нашел множество интересных вещей. Это была игровая компания, поэтому вещи были значительно менее «запоминающие», чем можно было найти где-либо еще. Это сделано для развлекательного чтения.

person Mark Storer    schedule 06.11.2019
comment
ВАРИАНТ 1 совсем не чистый. Это ужасный антипаттерн, который практически никогда не следует использовать. 1) Если конструктор бросает, у вас остается мертвый объект. 2) Даже если конструктор завершится успешно, не гарантируется, что myC будет правильным типом для создания вместо него! Учтите, что myC — это базовый класс, а this на самом деле указывает на производный тип. Этот подход, например, заменит Dog на Animal, и любой, кто все еще думает, что это Собака, окажется в серьезной беде. - person Chris Uzdavinis; 07.11.2019
comment
Отличный момент, хотя, честно говоря, деструктор не виртуальный, что заставляет меня поверить, что это не будет проблемой. Тогда проблема может быть решена путем обеспечения того, чтобы все дочерние классы реализовывали свой собственный оператор присваивания, который просто подвержен другому типу ошибок. Я не уверен, что даже RTTI смог бы вырыть для тебя яму номер один. - person Mark Storer; 07.11.2019
comment
Также прикольно, если на месте копи-ктор кидает. - person Deduplicator; 07.11.2019