Динамическое выделение массива объектов

Question

Это вопрос для начинающих, но я давно не занимался C++, так что...

У меня есть класс, который содержит динамически распределенный массив, скажем,

class A
{
    int* myArray;
    A()
    {
        myArray = 0;
    }
    A(int size)
    {
        myArray = new int[size];
    }
    ~A()
    {
        // Note that as per MikeB's helpful style critique, no need to check against 0.
        delete [] myArray;
    }
}

Но теперь я хочу создать динамически распределенный массив этих классов. Вот мой текущий код:

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    arrayOfAs[i] = A(3);
}

Но это ужасно взрывается. Потому что новый A объект создан (с A(3) вызов) разрушается, когда for итерация цикла завершается, а это означает, что внутренний myArray того, что A экземпляр получает delete []-ed.

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

• Создание конструктора копирования для A,
• С помощью vector<int> а также vector<A> так что мне не нужно беспокоиться обо всем этом.
• Вместо того, чтобы иметь arrayOfAs быть массивом A объекты, будь то массив A* указатели.

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

(Кроме того, оценка стиля приветствуется, поскольку я давно занимался C++.)

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

Answer 1

Для построения контейнеров вы, очевидно, хотите использовать один из стандартных контейнеров (например, std::vector). Но это прекрасный пример того, что вам нужно учитывать, когда ваш объект содержит RAW-указатели.

Если ваш объект имеет указатель RAW, вам нужно запомнить правило 3 (теперь правило 5 в C++11).

• Конструктор
• Destructor
• Копировать конструктор
• Оператор присваивания
• Конструктор Move (C++11)
• Назначение перемещения (C++11)

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

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

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

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

class A
{ 
    std::vector<int>   mArray;
    public:
        A(){}
        A(size_t s) :mArray(s)  {}
};

Глядя на вашу проблему:

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    // As you surmised the problem is on this line.
    arrayOfAs[i] = A(3);

    // What is happening:
    // 1) A(3) Build your A object (fine)
    // 2) A::operator=(A const&) is called to assign the value
    //    onto the result of the array access. Because you did
    //    not define this operator the compiler generated one is
    //    used.
}

Оператор присваивания, сгенерированный компилятором, подходит почти для всех ситуаций, но когда в игре используются RAW-указатели, вам нужно обратить внимание. В вашем случае это вызывает проблему из-за проблемы мелкого копирования. Вы получили два объекта, которые содержат указатели на один и тот же фрагмент памяти. Когда A(3) выходит из области видимости в конце цикла, он вызывает delete [] для своего указателя. Таким образом, другой объект (в массиве) теперь содержит указатель на память, которая была возвращена в систему.

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

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

Так что минимум для класса, который содержит указатель:

class A
{
    size_t     mSize;
    int*       mArray;
    public:
         // Simple constructor/destructor are obvious.
         A(size_t s = 0) {mSize=s;mArray = new int[mSize];}
        ~A()             {delete [] mArray;}

         // Copy constructor needs more work
         A(A const& copy)
         {
             mSize  = copy.mSize;
             mArray = new int[copy.mSize];

             // Don't need to worry about copying integers.
             // But if the object has a copy constructor then
             // it would also need to worry about throws from the copy constructor.
             std::copy(&copy.mArray[0],&copy.mArray[c.mSize],mArray);

         }

         // Define assignment operator in terms of the copy constructor
         // Modified: There is a slight twist to the copy swap idiom, that you can
         //           Remove the manual copy made by passing the rhs by value thus
         //           providing an implicit copy generated by the compiler.
         A& operator=(A rhs) // Pass by value (thus generating a copy)
         {
             rhs.swap(*this); // Now swap data with the copy.
                              // The rhs parameter will delete the array when it
                              // goes out of scope at the end of the function
             return *this;
         }
         void swap(A& s) noexcept
         {
             using std::swap;
             swap(this.mArray,s.mArray);
             swap(this.mSize ,s.mSize);
         }

         // C++11
         A(A&& src) noexcept
             : mSize(0)
             , mArray(NULL)
         {
             src.swap(*this);
         }
         A& operator=(A&& src) noexcept
         {
             src.swap(*this);     // You are moving the state of the src object
                                  // into this one. The state of the src object
                                  // after the move must be valid but indeterminate.
                                  //
                                  // The easiest way to do this is to swap the states
                                  // of the two objects.
                                  //
                                  // Note: Doing any operation on src after a move 
                                  // is risky (apart from destroy) until you put it 
                                  // into a specific state. Your object should have
                                  // appropriate methods for this.
                                  // 
                                  // Example: Assignment (operator = should work).
                                  //          std::vector() has clear() which sets
                                  //          a specific state without needing to
                                  //          know the current state.
             return *this;
         }   
 }

Answer 2

Я бы порекомендовал использовать std::vector: что-то вроде

typedef std::vector<int> A;
typedef std::vector<A> AS;

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

Answer 3

Конструктор вашего объекта A динамически размещает другой объект и сохраняет указатель на этот динамически размещенный объект в необработанном указателе.

Для этого сценария вы должны определить свой собственный конструктор копирования, оператор присваивания и деструктор. Сгенерированные компилятором не будут работать правильно. (Это является следствием "Закона Большой Тройки": классу с любым из деструктора, оператора присваивания, конструктора копирования обычно нужны все 3).

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

Альтернативой является сохранение указателя на ваш динамически выделенный int[] в другом объекте, который позаботится об этих вещах для вас. Что-то вроде vector<int> (как вы упомянули) или boost::shared_array<>,

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

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

Answer 4

1. Используйте массив или общий контейнер для объектов, только если они имеют конструкторы по умолчанию и копируют конструкторы.

2. В противном случае храните указатели (или умные указатели, но в этом случае могут возникать некоторые проблемы).

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

Answer 5

Вам нужен оператор присваивания, чтобы:

arrayOfAs[i] = A(3);

работает как надо.

Answer 6

Используя функцию размещения new Оператор, вы можете создать объект на месте и избежать копирования:

размещение (3):void* оператор new (размер std::size_t, void* ptr) noexcept;

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

Я предлагаю следующее:

A* arrayOfAs = new A[5]; //Allocate a block of memory for 5 objects
for (int i = 0; i < 5; ++i)
{
    //Do not allocate memory,
    //initialize an object in memory address provided by the pointer
    new (&arrayOfAs[i]) A(3);
}

Answer 7

Почему бы не иметь метод setSize.

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    arrayOfAs[i].SetSize(3);
}

Мне нравится "копия", но в этом случае конструктор по умолчанию ничего не делает. SetSize может копировать данные из исходного m_array (если он существует). Для этого вам нужно будет сохранить размер массива в классе.
ИЛИ ЖЕ
SetSize может удалить оригинальный m_array.

void SetSize(unsigned int p_newSize)
{
    //I don't care if it's null because delete is smart enough to deal with that.
    delete myArray;
    myArray = new int[p_newSize];
    ASSERT(myArray);
}