Pregunta

Esta es una pregunta para principiantes, pero hace mucho que no uso C++, así que allá va...

Tengo una clase que contiene una matriz asignada dinámicamente, digamos

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

Pero ahora quiero crear una matriz asignada dinámicamente de estas clases.Aquí está mi código actual:

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

Pero esto explota terriblemente.porque lo nuevo A objeto creado (con el A(3) llamada) se destruye cuando el for La iteración del bucle finaliza, y esto significa que el bucle interno myArray de eso A la instancia obtiene delete []-ed.

¿Entonces creo que mi sintaxis debe estar terriblemente equivocada?Supongo que hay algunas correcciones que parecen excesivas y espero evitarlas:

  • Creando un constructor de copia para A.
  • Usando vector<int> y vector<A> así que no tengo que preocuparme por todo esto.
  • En vez de tener arrayOfAs ser una serie de A objetos, que sea una serie de A* punteros.

Creo que esto es solo algo para principiantes donde hay una sintaxis que realmente funciona cuando se intenta asignar dinámicamente una serie de cosas que tienen una asignación dinámica interna.

(Además, se agradecen las críticas de estilo, ya que ha pasado un tiempo desde que hice C++).

Actualización para futuros espectadores:Todas las respuestas a continuación son realmente útiles.Se acepta el de Martin debido al código de ejemplo y la útil "regla de 4", pero realmente sugiero leerlos todos.Algunas son declaraciones buenas y concisas de lo que está mal y otras señalan correctamente cómo y por qué. vectors son un buen camino a seguir.

¿Fue útil?

Solución

Para construir contenedores, obviamente desea utilizar uno de los contenedores estándar (como std :: vector). Pero este es un ejemplo perfecto de las cosas que debe tener en cuenta cuando su objeto contiene punteros RAW.

Si su objeto tiene un puntero RAW, entonces debe recordar la regla de 3 (ahora la regla de 5 en C ++ 11).

  • Constructor
  • Destructor
  • Copiar constructor
  • Operador de asignación
  • Mover constructor (C ++ 11)
  • Asignación de movimiento (C ++ 11)

Esto se debe a que si no se define, el compilador generará su propia versión de estos métodos (ver más abajo). Las versiones generadas por el compilador no siempre son útiles cuando se trata de punteros RAW.

El constructor de copias es el difícil de corregir (no es trivial si desea proporcionar la garantía de excepción fuerte). El operador de asignación se puede definir en términos del constructor de copia, ya que puede utilizar el idioma de copiar e intercambiar internamente.

Vea a continuación todos los detalles sobre el mínimo absoluto para una clase que contiene un puntero a una matriz de enteros.

Sabiendo que no es trivial corregirlo, debería considerar usar std :: vector en lugar de un puntero a una matriz de enteros. El vector es fácil de usar (y expandir) y cubre todos los problemas asociados con las excepciones. Compare la siguiente clase con la definición de A a continuación.

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

Analizando su problema:

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

El operador de asignación generado por el compilador está bien para casi todas las situaciones, pero cuando los punteros RAW están en juego, debe prestar atención. En su caso, está causando un problema debido al problema de copia superficial . Ha terminado con dos objetos que contienen punteros a la misma pieza de memoria. Cuando A (3) se sale del alcance al final del ciclo, llama a delete [] en su puntero. Por lo tanto, el otro objeto (en la matriz) ahora contiene un puntero a la memoria que se ha devuelto al sistema.

El compilador generó el constructor de copias ; copia cada variable miembro utilizando el constructor de copia de miembros. Para los punteros, esto solo significa que el valor del puntero se copia del objeto de origen al objeto de destino (por lo tanto, copia superficial).

El operador de asignación generado por el compilador ; copia cada variable miembro utilizando ese operador de asignación de miembros. Para los punteros, esto solo significa que el valor del puntero se copia del objeto de origen al objeto de destino (por lo tanto, copia superficial).

Entonces, el mínimo para una clase que contiene un puntero:

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

Otros consejos

Recomiendo usar std :: vector: algo así como

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

No hay nada de malo con la ligera exageración de STL, y podrás pasar más tiempo implementando las características específicas de tu aplicación en lugar de reinventar la bicicleta.

El constructor de su objeto A asigna otro objeto dinámicamente y almacena un puntero a ese objeto asignado dinámicamente en un puntero sin formato.

Para ese escenario, debe debe definir su propio constructor de copia, operador de asignación y destructor. Los compiladores generados no funcionarán correctamente. (Esto es un corolario de la & Quot; Ley de los Tres Grandes & Quot ;: Una clase con cualquiera de destructor, operador de asignación, constructor de copia generalmente necesita los 3).

Ha definido su propio destructor (y mencionó la creación de un constructor de copia), pero necesita definir los otros 2 de los tres grandes.

Una alternativa es almacenar el puntero a su int[] asignado dinámicamente en algún otro objeto que se encargará de estas cosas por usted. Algo así como un vector<int> (como mencionó) o un boost::shared_array<>.

Para reducir esto, para aprovechar RAII en toda su extensión, debe evitar tratar con punteros sin procesar en la medida de lo posible.

Y dado que solicitó otras críticas de estilo, una menor es que cuando elimina punteros sin procesar no necesita verificar 0 antes de llamar a delete - <=> maneja ese caso al no hacer nada para que no lo haga ' No tiene que abarrotar su código con los cheques.

  1. Utilice la matriz o el contenedor común para los objetos solo si tienen constructores predeterminados y de copia.

  2. Almacene los punteros de otra manera (o punteros inteligentes, pero puede encontrar algunos problemas en este caso).

PD: siempre defina constructores propios predeterminados y de copia, de lo contrario se utilizarán los generados automáticamente

Necesita un operador de asignación para que:

arrayOfAs[i] = A(3);

funciona como debería.

¿Por qué no tener un método setSize?

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

Me gusta la " copy " pero en este caso, el constructor predeterminado realmente no está haciendo nada. El SetSize podría copiar los datos del m_array original (si existe). Debería almacenar el tamaño de la matriz dentro de la clase para hacerlo.
O
SetSize podría eliminar el m_array original.

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

Usando la función de colocación del operador new, puede crear el objeto en su lugar y evitar copiar:

  

colocación (3): void * operador nuevo (std :: size_t size, void * ptr) noexcept;

     

Simplemente devuelve ptr (no se asigna almacenamiento).   Sin embargo, tenga en cuenta que, si una nueva expresión llama a la función, se realizará la inicialización adecuada (para los objetos de clase, esto incluye llamar a su constructor predeterminado).

Sugiero lo siguiente:

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);
}
Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top