Проверить «самопредоставление» в конструкторе копирования? - PullRequest
16 голосов
/ 01 апреля 2011

Сегодня в университете профессор рекомендовал мне проверить (this != &copy) в конструкторе копирования, аналогично тому, как вы должны это делать при перегрузке operator=. Однако я задал этот вопрос, потому что не могу вспомнить ни одной ситуации, когда this был бы равен аргументу при создании объекта.

Он признал, что я сделал хорошее замечание. Итак, мой вопрос: имеет ли смысл проводить эту проверку, или это невозможно, чтобы это облажалось?

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

Edit2 : Test a(a) компилируется в MinGW, но не в MSVS10. Test a = a компилируется на обоих, поэтому я предполагаю, что gcc будет вести себя примерно одинаково. К сожалению, VS не показывает отладочное сообщение с «Переменная используется без инициализации» . Тем не менее, он правильно показывает это сообщение для int i = i. Может ли это на самом деле считаться недостатком языка с ++?

class Test
{
   Test(const Test &copy)
   {
      if (this != &copy) // <-- this line: yay or nay?
      {
      }
   }
   Test &operator=(const Test &rhd)
   {
      if (this != &rhd) // <-- in this case, it makes sense
      {
      }
   }
};

Ответы [ 9 ]

9 голосов
/ 01 апреля 2011

Лично я думаю, что ваш профессор не прав, и вот почему.

Конечно, код скомпилируется.И конечно, код не работает.Но это так далеко, как ваш Проф пошел со своими рассуждениями, и затем он приходит к выводу: «О, хорошо, мы должны посмотреть, если мы самоопределяем, и если мы это сделаем, просто вернитесь».

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

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

9 голосов
/ 01 апреля 2011

Это допустимый C ++ и вызывает конструктор копирования:

Test a = a;

Но это не имеет смысла, потому что a используется до его инициализации.

5 голосов
/ 02 апреля 2011

Если вы хотите быть параноиком, тогда:

class Test
{
   Test(const Test &copy)
   {
       assert(this != &copy);
       // ...
   }
};

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

4 голосов
/ 01 апреля 2011

Ваш инструктор, вероятно, пытается избежать этой ситуации -

#include <iostream>

class foo
{
    public:
    foo( const foo& temp )
    {
        if( this != &temp )
        std::cout << "Copy constructor \n";
    }
};

int main()
{
    foo obj(obj);  // This is any how meaning less because to construct
                   // "obj", the statement is passing "obj" itself as the argument

}

Поскольку имя (то есть obj) отображается в объявлении времени, код компилируется и является действительным.

3 голосов
/ 01 апреля 2011

Ваш инструктор может подумать о проверке самоназначения в операторе copy assignment .

Рекомендуется проверять самоназначение в операторе присваивания как у Саттера, так и у Александреску.«Стандарты кодирования C ++» и ранее «Эффективный C ++» Скотта Мейера.

1 голос
/ 11 мая 2013

В нормальных ситуациях кажется, что здесь нет необходимости. Но рассмотрим следующую ситуацию:

class A{ 
   char *name ; 
public:
   A & operator=(const A & rhs);
};

A & A::operator=(const A &rhs){
   name = (char *) malloc(strlen(rhs.name)+1);
   if(name) 
      strcpy(name,rhs.name);
   return *this;
}

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

A & A::operator=(const A &rhs){
     if(this != &rhs){
       name = (char *) malloc(strlen(rhs.name)+1);
       if(name) 
          strcpy(name,rhs.name);
     } 
   return *this;
}
0 голосов
/ 07 декабря 2016

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

Test a;
a = a;

Например,

const Test& copy(const Test& that) {
    if (this == &that) {
        return *this
    }
    //otherwise construct new object and copy over
}
Test(const &that) {
    copy(that);
}

Test& operator=(const Test& that) {
    if (this != &that) { //not checking self 
        this->~Test();
    }
    copy(that);
 }

Выше, когда выполняется a = a, вызывается перегрузка оператора, которая вызывает функцию копирования, которая затем обнаруживает самоприсвоение,

0 голосов
/ 29 апреля 2016

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

class Test
{    
    **public:**
    Test(const Test& obj )
    {
       size = obj.size;
       a = new int[size];   
    }

    ~Test()
    {....}

    void display()
    {
        cout<<"Constructor is valid"<<endl;
    }
    **private:**

 }

Когда я создал конструктор копирования и вызвал функцию-член, я не сделал

 Test t2(t2);
 t2.display(); 

Выход:
Внутренний конструктор по умолчанию
Внутри параметризованный конструктор
Конструктор внутренних копий
Конструктор действителен

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

    Test(const Test& obj )
    {
        if(this != &obj )
        {
            size = obj.size;
            a = new int[size];
        }
    }

Ошибка выполнения:
Ошибка в `/ home / bot / 1eb372c9a09bb3f6c19a27e8de801811 ': munmap_chunk (): неверный указатель: 0x0000000000400dc0

0 голосов
/ 02 апреля 2011

При записи операторов присваивания и конструкторов копирования всегда делают следующее:

struct MyClass
{
    MyClass(const MyClass& x)
    {
        // Implement copy constructor. Don't check for
        // self assignment, since &x is never equal to *this.
    }

    void swap(MyClass& x) throw()
    {
        // Implement lightweight swap. Swap pointers, not contents.
    }

    MyClass& operator=(MyClass x)
    {
        x.swap(*this); return *this;
    }
};

При передаче значения x в оператор присваивания выполняется копия.Затем вы меняете его на *this, и деструктор x вызывается при возврате со старым значением *this.Простой, элегантный, безопасный для исключений , без дублирования кода и без необходимости самопроверкибезопасность исключений (и пока игнорируем спецификатор throw() для swap).

...