Должен ли я использовать размещение нового в операторе назначения копирования - PullRequest
1 голос
/ 06 ноября 2019

Я создаю дискриминационный подобный союзу класс. Я использую c ++ 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;
}

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

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 напрямую, не убедившись, что строка там когда-либо была инициализирована, и она вылетела.

1 Ответ

0 голосов
/ 07 ноября 2019

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

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

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

1)

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

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

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

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

  • пропускал бы все, что требовало вызова деструктора производного класса.
  • превращал производный класс в экземпляр MyC. В ОСНОВНОМ. Я подозреваю, что производный деструктор по-прежнему будет вызываться, и за ним может последовать большая опасность
    • ОТО, это могло бы просто очистить весь материал, который «просочился», когда мы ранее назвали неправильный деструктор. Это действительно просто зависит.

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

2)

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

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

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

...