Domanda

Questa è una specie di domanda per principianti, ma non faccio C ++ da molto tempo, quindi ecco qui ...

Ho una classe che contiene un array allocato dinamicamente, diciamo

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

Ma ora voglio creare una matrice allocata dinamicamente di queste classi. Ecco il mio codice attuale:

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

Ma questo esplode terribilmente. Perché il nuovo A oggetto creato (con la A(3) chiamata) viene distrutto quando termina l'iterazione del ciclo for e ciò significa che myArray interno di tale delete [] istanza ottiene vector<int> - ed.

Quindi penso che la mia sintassi debba essere terribilmente sbagliata? Immagino che ci siano alcune correzioni che sembrano eccessive, che spero di evitare:

  • Creazione di un costruttore di copie per vector<A>.
  • Uso di arrayOfAs e A*, quindi non devo preoccuparmi di tutto questo.
  • Invece di avere vector essere un array di <=> oggetti, deve essere un array di <=> puntatori.

Penso che questa sia solo una cosa per principianti in cui esiste una sintassi che funziona effettivamente quando si tenta di allocare dinamicamente una matrice di cose che hanno allocazione dinamica interna.

(Inoltre, le critiche sullo stile sono state apprezzate, dato che è passato un po 'di tempo da quando ho fatto C ++.)

Aggiornamento per futuri spettatori : tutte le risposte di seguito sono davvero utili. Martin è accettato a causa del codice di esempio e dell'utile & Quot; regola di 4, & Quot; ma suggerisco davvero di leggerli tutti. Alcuni sono affermazioni positive e sintetiche di ciò che è sbagliato, e alcuni sottolineano correttamente come e perché <=> s sono un buon modo per andare.

È stato utile?

Soluzione

Per la costruzione di container, ovviamente, si desidera utilizzare uno dei container standard (come std :: vector). Ma questo è un esempio perfetto delle cose che devi considerare quando il tuo oggetto contiene puntatori RAW.

Se il tuo oggetto ha un puntatore RAW, devi ricordare la regola 3 (ora la regola 5 in C ++ 11).

  • Constructor
  • Destructor
  • Copia costruttore
  • Operatore di assegnazione
  • Sposta costruttore (C ++ 11)
  • Sposta assegnazione (C ++ 11)

Questo perché, se non definito, il compilatore genererà la propria versione di questi metodi (vedi sotto). Le versioni generate dal compilatore non sono sempre utili quando si tratta di puntatori RAW.

Il costruttore di copie è quello difficile da correggere (non è banale se si desidera fornire la forte garanzia di eccezione). L'operatore di assegnazione può essere definito in termini di costruttore di copie in quanto è possibile utilizzare il linguaggio di copia e scambio internamente.

Vedi sotto per i dettagli completi sul minimo assoluto per una classe che contiene un puntatore a un array di numeri interi.

Sapendo che non è banale farlo correttamente, dovresti considerare l'utilizzo di std :: vector piuttosto che un puntatore a un array di numeri interi. Il vettore è facile da usare (ed espandere) e copre tutti i problemi associati alle eccezioni. Confronta la seguente classe con la definizione di A in basso.

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

Guardando il tuo 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.
}

L'operatore di assegnazione generato dal compilatore va bene per quasi tutte le situazioni, ma quando i puntatori RAW sono in gioco è necessario prestare attenzione. Nel tuo caso sta causando un problema a causa del problema copia superficiale . Hai finito con due oggetti che contengono puntatori allo stesso pezzo di memoria. Quando A (3) esce dall'ambito alla fine del loop, chiama delete [] sul suo puntatore. Quindi l'altro oggetto (nell'array) ora contiene un puntatore alla memoria che è stato restituito al sistema.

Il compilatore ha generato il costruttore di copie ; copia ogni variabile del membro usando quel costruttore di copia dei membri. Per i puntatori questo significa solo che il valore del puntatore viene copiato dall'oggetto di origine all'oggetto di destinazione (quindi copia superficiale).

L'operatore di assegnazione generato dal compilatore ; copia ogni variabile del membro usando quell'operatore di assegnazione dei membri. Per i puntatori questo significa solo che il valore del puntatore viene copiato dall'oggetto di origine all'oggetto di destinazione (quindi copia superficiale).

Quindi il minimo per una classe che contiene un puntatore:

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

Altri suggerimenti

Consiglio di usare std :: vector: qualcosa come

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

Non c'è niente di sbagliato nel leggero eccesso di STL e sarai in grado di dedicare più tempo all'implementazione delle funzionalità specifiche della tua app invece di reinventare la bicicletta.

Il costruttore del tuo oggetto A alloca dinamicamente un altro oggetto e memorizza un puntatore a quell'oggetto allocato dinamicamente in un puntatore non elaborato.

Per quello scenario, devi definire il tuo costruttore di copie, operatore di assegnazione e distruttore. Quelli generati dal compilatore non funzioneranno correttamente. (Questo è un corollario della & Quot; Legge dei tre grandi & Quot ;: Una classe con uno qualsiasi di distruttore, operatore di assegnazione, costruttore di copie generalmente richiede tutti e 3).

Hai definito il tuo distruttore (e hai menzionato la creazione di un costruttore di copie), ma devi definire entrambi gli altri 2 dei tre grandi.

Un'alternativa è quella di memorizzare il puntatore al tuo int[] allocato dinamicamente in qualche altro oggetto che si occuperà di queste cose per te. Qualcosa come un vector<int> (come hai già detto) o un boost::shared_array<>.

Per farla breve: per sfruttare appieno RAII nella massima misura, dovresti evitare di trattare i puntatori grezzi nella misura del possibile.

E poiché hai richiesto altre critiche di stile, una minore è che quando elimini i puntatori non elaborati non devi controllare 0 prima di chiamare delete - <=> gestisce quel caso senza fare nulla, quindi non " devo ingombrare il codice con i controlli.

  1. Usa array o container comuni per oggetti solo se hanno costruttori predefiniti e copia.

  2. Memorizza i puntatori altrimenti (o i puntatori intelligenti, ma in questo caso potrebbero verificarsi alcuni problemi).

PS: definire sempre i propri predefiniti e copiare i costruttori, altrimenti verranno utilizzati automaticamente generati

È necessario un operatore di assegnazione in modo che:

arrayOfAs[i] = A(3);

funziona come dovrebbe.

Perché non avere un metodo setSize.

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

Mi piace il " copy " ma in questo caso il costruttore predefinito non sta realmente facendo nulla. SetSize potrebbe copiare i dati dall'array originale m (se esiste). Dovresti archiviare le dimensioni dell'array all'interno della classe per farlo.
O
SetSize potrebbe eliminare l'originale 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);
}

Utilizzando la funzione di posizionamento dell'operatore new, è possibile creare l'oggetto in posizione ed evitare la copia:

  

posizionamento (3): void * operator new (std :: size_t size, void * ptr) noexcept;

     

Restituisce semplicemente ptr (non viene allocato spazio di archiviazione).   Si noti tuttavia che, se la funzione viene chiamata da una nuova espressione, verrà eseguita l'inizializzazione corretta (per gli oggetti di classe, questo include chiamare il suo costruttore predefinito).

Suggerisco quanto segue:

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);
}
Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top