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

https://stackoverflow.com/questions/255612

05-07-2019
|

Вопрос

Это своего рода вопрос для начинающих, но я давно не занимался 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 []-эд.

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

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

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

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

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

Решение

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

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

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

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

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

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

Зная, что получить его правильно нетривиально, вам следует рассмотреть возможность использования 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.
}

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

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

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

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

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;
         }   
 }

Другие советы

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

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

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

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

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

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

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

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

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

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

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

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

arrayOfAs[i] = A(3);

работает так, как и должно быть.

Почему бы не использовать метод 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);
}

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

размещение (3) :void* оператор new (std::size_t size, void* ptr) без исключения;

Просто возвращает 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);
}

Лицензировано под: CC-BY-SA с атрибуция

Не связан с StackOverflow