Как делать не надо

[Romeo]

Конструктор копирования должен копировать, для чего необходим код, не повторяющий код дефолтного конструктора, так как дефолтный не копирует. Повторять дефолтный конструктор в лучшем случае должна лишь часть кода. Или под some copy code понимается не копия кода, а копирующий код? Да и синтаксис перевран: копирующий конструктор должен принимать константную ссылку, а не просто объект, иначе он получит уже копию, а её то он и должен изготовить.

Под some copy code подразумевается именно правильный копирующий код. Он был свёрнут в комментарий для того, чтобы показать, что проблема точно не в нём.

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

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

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

[Romeo]

Не обновляется размер массива.

Не обновляется размер массива - верное замечание. Но это моя личная ошибка, так как я писал зарисовку прямо в блокноте. Код поправил.

И строка *(CArray::GetShared()) = *pCurArray; для такого прототипа GetShared какая то странная. Она вообще скомпилируется?

Не знаю, что тебя в этой строке смущает. Обычное разыменование. Конечно скомпилируется.

Проблема не найдена. Кому интересно ищите. Оксалайа уже прислала правильный ответ в личку.

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

[Сионист]

Обычное разыменование.

Не обычное, так будет правильней. void разыменовываться не обязан.

[Romeo]

Не обычное, так будет правильней. void разыменовываться не обязан.

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

В общем, проблема точно не в этом. Ищем.

[Сионист]

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

[Romeo]

Да, проблема именно в этом.

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

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

Код: Выделить всё

   CArray& operator=(const CArray& rhs)
   {
      if (&rhs != this)
      {
         delete [] m_pArray;
         m_pArray = new int [m_nSize = rhs.m_nSize];
         memcpy(m_pArray, rhs.m_pArray, rhs.m_nSize * sizeof(*m_pArray));
      }
      return *this;
   }

Чуть позже создам ещё пару отдельных темок с занимательными задачками.

[Сионист]

Неверно. Код, который в твоём примере скрыт в трёх точках будет выполняться КАЖДЫЙ раз при вызове f.

Ссылка же не на функцию.

Код: Выделить всё

#include <iostream>
#include <string>
int &f()
{
 static int x;
 std::cout<<"f"<<std::endl;
 return x;
}
int main ()
{
 int &y=f();
 y=2;
 y=4;
 return 0;
}

вывело f только один раз, а не 3.

[Сионист]

Всегда, когда описываете свой оператор присваивания делайте проверку на присваивание самому себе.

Свой оператор ещё не означает обязательного освобождения динамической памяти.

Код: Выделить всё

class A
{
 int *Data;
 public:
  A ()
  {
   Data=new int [1000000];
  };
 ~A ()
 {
  delete [] Data;
 }
 A (const A &a)
 {
  int *p1;
  int *p2;
  Data=new int [1000000];
  for (p2=Data+999999, p1=a.Data+999999; p2>=Data; --p1, --p2)
  {
   *p2=*p1;
  }
 }
 A operator = (const A &a);
 {
  int *p1;
  int *p2;
  for (p2=Data+999999, p1=a.Data+999999; p2>=Data; --p1, --p2)
  {
   *p2=*p1;
  }
  return *this;
 }
 ...
};
...
A a;
a=a;

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

[Romeo]

Ссылка же не на функцию.

Конечно ссылка не на функцию. У меня и подумать бы не получилось о таком.

Ты подобной фразой пытаешься поставить под сомнение тот факт, что три точки выполняются каждый раз при вызове функции?

Вот программа:

Код: Выделить всё

#include <iostream>

int& f()
{
   static int x;
   std::cout << "f ";
   return x;
}

int main()
{
   for (int i = 0; i < 5; ++i)
   {
      int& y = f();
   }   
   return 0;
}

Вот результат запуска:

f f f f f

Может ты путаешь с тем, однократно происходит инициализация статической переменной? Но в твоём примере инициализация вообще не делается. Это явно видно, так как объявление переменной x заканчивается точкой с запятой и знак присваивания отсутствует.

[Romeo]

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

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