Это плохая форма для вызова оператора присваивания по умолчанию из конструктора копирования? - PullRequest
12 голосов
/ 07 октября 2009

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

Это плохая форма вызова оператора присваивания по умолчанию из конструктора копирования?

Оператор присваивания по умолчанию будет хорошо работать с Plain Old Data (int, double, char, short), а также с классами, определенными пользователем, для их операторов присваивания. Указатели должны обрабатываться отдельно.

Один недостаток заключается в том, что этот метод делает оператор присваивания искалеченным, поскольку дополнительная повторная инициализация не выполняется. Также невозможно отключить использование оператора присваивания, открывая, таким образом, возможность пользователю создавать поврежденный класс с помощью неполного оператора присваивания по умолчанию A obj1,obj2; obj2=obj1; /* Could result is an incorrectly initialized obj2 */.

Было бы хорошо смягчить требование, чтобы в a(orig.a),b(orig.b)... помимо a(0),b(0) ... было написано. Необходимость писать всю инициализацию дважды создает два места для ошибок, и если новые переменные (скажем, double x,y,z) должны были быть добавлены в класс, код инициализации должен был бы быть правильно добавлен как минимум в 2 местах вместо 1.

Есть ли лучший способ?

Есть ли лучший способ в C ++ 0x?

class A {
  public:
    A(): a(0),b(0),c(0),d(0)
    A(const A & orig){
      *this = orig;       /* <----- is this "bad"? */
      c = int();
    }
  public:
    int a,b,c,d;
};

A X;
X.a = 123;
X.b = 456;
X.c = 789;
X.d = 987;

A Y(X);

printf("X: %d %d %d %d\n",X.a,X.b,X.c,X.d);
printf("Y: %d %d %d %d\n",Y.a,Y.b,Y.c,Y.d);

Выход:

X: 123 456 789 987
Y: 123 456 0 987

Альтернативный конструктор копирования:

A(const A & orig):a(orig.a),b(orig.b),c(0),d(orig.d){}  /* <-- is this "better"? */

Ответы [ 6 ]

13 голосов
/ 07 октября 2009

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

T& T::operator=(T t) {
    swap(*this, t);
    return *this;
}

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

Во-первых, параметр t преднамеренно передается по значению, так что будет вызван конструктор копирования ( большую часть времени ), и мы можем изменить его в соответствии с содержанием нашего сердца, не влияя на исходное значение , Использование const T& не сможет скомпилироваться, а T& вызовет неожиданное поведение, изменив значение назначенного элемента.

Этот метод также требует, чтобы swap был специализирован для типа таким образом, чтобы не использовать оператор присваивания типа (как это делает std::swap), иначе это вызовет бесконечную рекурсию. Будьте осторожны с любыми паразитами using std::swap или using namespace std, так как они вытянут std::swap в область и вызовут проблемы, если вы не специализировали swap для T. Разрешение перегрузки и ADL обеспечат правильную версию подкачки, если вы ее определили.

Есть несколько способов определить swap для типа. Первый метод использует swap функцию-член для выполнения фактической работы и имеет специализацию swap, которая ему делегируется, например:

class T {
public:
    // ....
    void swap(T&) { ... }
};

void swap(T& a, T& b) { a.swap(b); }

Это довольно часто встречается в стандартной библиотеке; std::vector, например, подкачка реализована таким образом. Если у вас есть функция-член swap, вы можете просто вызвать ее непосредственно из оператора присваивания и избежать проблем с поиском функции.

Другой способ - объявить swap в качестве функции-друга и заставить ее выполнять всю работу:

class T {
    // ....
    friend void swap(T& a, T& b);
};

void swap(T& a, T& b) { ... }

Я предпочитаю второй, так как swap() обычно не является неотъемлемой частью интерфейса класса; кажется более подходящим в качестве свободной функции. Однако дело вкуса.

Наличие оптимизированного swap для типа является распространенным методом достижения некоторых преимуществ ссылок на rvalue в C ++ 0x, поэтому в целом неплохо, если класс может воспользоваться этим и вам действительно нужно производительность.

4 голосов
/ 07 октября 2009

В вашей версии конструктора копирования члены сначала создаются по умолчанию, а затем назначаются.
С целочисленными типами это не имеет значения, но если у вас были нетривиальные члены, такие как std::string s, это ненужные накладные расходы.

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

2 голосов
/ 08 октября 2009

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

Гораздо лучше было бы использовать идиому копирования и замены и указать, какие члены не следует копировать в конструкторе копирования. У вас все еще есть место, где выражается поведение «не копировать этого члена», но теперь ваш оператор присваивания и конструктор копирования согласованы.

class A
{
public:

    A() : a(), b(), c(), d() {}

    A(const A& other)
        : a(other.a)
        , b(other.b)
        , c() // c isn't copied!
        , d(other.d)

    A& operator=(const A& other)
    {
        A tmp(other); // doesn't copy other.c
        swap(tmp);
        return *this;
    }

    void Swap(A& other)
    {
        using std::swap;
        swap(a, other.a);
        swap(b, other.b);
        swap(c, other.c); // see note
        swap(d, other.d);
    }

private:
    // ...
};

Примечание: в функции члена swap, я поменял элемент c. Для целей использования в операторе присваивания это сохраняет поведение, совпадающее с поведением конструктора копирования: оно повторно инициализирует член c. Если вы оставляете функцию swap общедоступной или предоставляете доступ к ней через бесплатную функцию swap, вы должны убедиться, что это поведение подходит для других применений подкачки.

1 голос
/ 08 октября 2009

Лично я считаю, что нарушенный оператор присваивания является убийцей. Я всегда говорю, что люди должны читать документацию и не делать ничего, о чем она им не говорит, но даже при этом слишком просто написать назначение, не думая об этом, или использовать шаблон, который требует присваиваемого типа. Есть причина для не копируемой идиомы: если operator= не сработает, просто слишком опасно оставлять его доступным.

Если я правильно помню, C ++ 0x позволит вам сделать это:

private:
    A &operator=(const A &) = default;

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

0 голосов
/ 08 октября 2009

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

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

Если вам по какой-то причине необходимо предоставить конструктор мелкой копии по умолчанию, то в C ++ 0X есть

 X(const X&) = default;

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

0 голосов
/ 08 октября 2009

Я бы назвал это плохой формой не потому, что вы дважды присваиваете все свои объекты, а потому, что, по моему опыту, это часто плохая форма для полагаться на конструктор / оператор копирования по умолчанию для определенного набора функциональность. Поскольку их нет нигде в источнике, трудно сказать, что желаемое поведение зависит от их поведения. Например, что если кто-то через год захочет добавить вектор строк в ваш класс? У вас больше нет простых старых типов данных, и сопровождающему будет очень трудно понять, что они ломают вещи.

Я думаю, что, как бы ни было СУХО, создавать тонкие неуказанные требования намного хуже с точки зрения обслуживания. Даже повторение себя, как бы плохо это ни было, является меньшим злом.

...