Динамическое выделение массива объектов - PullRequest
53 голосов
/ 01 ноября 2008

Это вопрос для начинающих, но я давно не занимался 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 s - хороший путь.

Ответы [ 7 ]

117 голосов
/ 01 ноября 2008

Для построения контейнеров вы, очевидно, хотите использовать один из стандартных контейнеров (например, 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-указатели, вам следует обратить внимание В вашем случае это вызывает проблему из-за проблемы shallow copy . Вы получили два объекта, которые содержат указатели на один и тот же фрагмент памяти. Когда 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;
         }   
 }
10 голосов
/ 01 ноября 2008

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

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

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

6 голосов
/ 01 ноября 2008

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

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

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

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

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

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

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

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

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

2 голосов
/ 01 ноября 2008

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

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

arrayOfAs[i] = A(3);

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

1 голос
/ 22 апреля 2016

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

place (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);
}
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...