2012-01-14 12 views
7

Sto usando questo due classiC++, come copiare correttamente std :: vector <Class *> nel costruttore di copie?

// This is generic data structure containing some binary data 
class A { 
public: 
    A(); 
    A(const A&); 
    ~A(); 
} 

// Main data container 
class B { 
public: 
    B(); 
    B(const B&); 
    ~B(); 
protected: 
    std::vector<A *> data; 
} 

// Copy constructor for class b 
B::B(const B& orig):data() { 
    for(std::vector<A *>::const_iterator it = orig.data.begin(); 
     it < orig.data.end(); ++it){ 
     data.push_back(new A(*(*it))); 
    } 
} 

Credo che questa classe avrebbe fatto lavoro, ma sto trovando per strada come raggiungere la perfezione totale in questo.

Inizialmente :data() - questa inizializzazione è necessaria per inizializzare correttamente il vettore vuoto (ed è parte della scrittura di un codice buono e pulito)?

Come utilizzare vector::iterator nel costruttore di copie, l'unico modo che ho trovato è quello che ho scritto nel codice (const dovrebbe essere obbligatorio per il costruttore di copie).

La copia solo vettoriale copiava solo i valori del puntatore e non interi oggetti?

E infine l'inizializzazione di nuovi dati ... Esiste un modo per sostituire l'intero ciclo con un pezzo di codice più piccolo e/o esiste un metodo standard per scrivere il costruttore di copia per std :: contenitori che contiene puntatori di oggetti?

domanda secondaria: sto assumendo utilizzando vector<A *> è molto più adatto ed efficace per vari motivi che solo vector<A> (non copiare ogni volta, il potere di decidere se (non) per copiare gli oggetti ...)

+0

Intendevi "Domanda secondaria: presumo l'utilizzo di un vettore di puntatori ..." –

+0

Prealloca' dati' nella lista di inizializzazione.Utilizzo 'push_back()' come quello è molto inefficace – lapk

+0

Risposta secondaria: Penso che, se non si possono usare i puntatori, non si dovrebbero usare. – Lol4t0

risposta

9

Il data() non è necessario perché verrà eseguito automaticamente sul vettore prima che venga inserito il costruttore. È necessario solo inizializzare i membri che sono tipi o tipi di POD che non hanno un costruttore predefinito (o riferimenti, costanti, ecc.).

È possibile inizializzare il vettore con il numero di elementi dell'altro, in modo che il vettore non debba ridimensionarsi man mano che cresce. Se non lo fai, inizi con un piccolo vettore e raggiungilo in modo incrementale con le allocazioni e le riallocazioni. Questo renderà il vettore il formato corretto fin dall'inizio:

B::B(const B& orig) : data(orig.data.size()) { 
    for (std::size_t i = 0; i < orig.data.size(); ++i) 
     data[i] = new A(*orig.data[i]); 
} 

Si noti che non si utilizza push_back più perché il vettore è già pieno di orig.data.size() numero di elementi che sono di default costruito (che è NULL nel caso di puntatori).

Questo riduce anche il codice perché è possibile utilizzare un numero intero per iterarlo anziché un iteratore.

Se davvero si vuole usare iteratori, si può fare

B::B(const B& orig) : data(orig.data.size()) { 
    // auto is preferable here but I don't know if your compiler supports it 
    vector<A*>::iterator thisit = data.begin(); 
    vector<A*>::const_iterator thatit = orig.data.cbegin(); 

    for (; thatit != orig.data.cend(); ++thisit, ++thatit) 
     *thisit = new A(**thatit); 
} 

Il vantaggio di questo è che che possa funzionare con altri tipi di contenitori (come list) semplicemente cambiando i tipi di iteratori (ma di Ovviamente andrebbe via se hai auto).

Se si desidera aggiungere un'eccezione di sicurezza, è necessario un blocco try/catch:

B::B(const B& orig) : data(orig.data.size()) { 
    try { 
     // auto is preferable here but I don't know if your compiler supports it 
     vector<A*>::iterator thisit = data.begin(); 
     vector<A*>::const_iterator thatit = orig.data.cbegin(); 

     for (; thatit != orig.data.cend(); ++thisit, ++thatit) 
      *thisit = new A(**thatit); 
    } catch (...) { 
     for (vector<A*>::iterator i = data.begin(); i != data.end(); ++i) 
      if (!*i) 
       break; 
      else 
       delete *i; 

     throw; 
    } 
} 

In questo modo non si avrà una perdita di memoria se uno dei new chiamate genera un'eccezione. Ovviamente è possibile utilizzare il try/catch insieme alla modalità senza iteratori se preferisci farlo in questo modo.

+1

È necessario dereferenziare' orig. dati [i] '. – someguy

+0

@someguy whoops, hai ragione, corretto. –

+2

Un'altra cosa da prendere in considerazione è la gestione degli errori: cosa succede se uno dei' new's fai l nel mezzo del ciclo? Probabilmente il programma terminerà, ma se l'errore di memoria esaurita viene gestito più avanti nello stack di chiamate, si verificherà una perdita di memoria. –

Problemi correlati