2016-06-20 5 views
12

Ho visto molti codice implementare la regola del cinque in termini di copia e scambio, ma penso che possiamo usare una funzione di spostamento per sostituire la funzione di scambio come in il seguente codice:Un modo migliore per implementare l'idioma copy-and-swap in C++ 11

#include <algorithm> 
#include <cstddef> 

class DumbArray { 
public: 
    DumbArray(std::size_t size = 0) 
     : size_(size), array_(size_ ? new int[size_]() : nullptr) { 
    } 

    DumbArray(const DumbArray& that) 
     : size_(that.size_), array_(size_ ? new int[size_] : nullptr) { 
     std::copy(that.array_, that.array_ + size_, array_); 
    } 

    DumbArray(DumbArray&& that) : DumbArray() { 
     move_to_this(that); 
    } 

    ~DumbArray() { 
     delete [] array_; 
    } 

    DumbArray& operator=(DumbArray that) { 
     move_to_this(that); 
     return *this; 
    } 

private: 
    void move_to_this(DumbArray &that) { 
     delete [] array_; 
     array_ = that.array_; 
     size_ = that.size_; 
     that.array_ = nullptr; 
     that.size_ = 0; 
    } 

private: 
    std::size_t size_; 
    int* array_; 
}; 

Questo codice, penso

  1. Eccezione sicuro
  2. richiedono meno di battitura, come molti la funzione basta chiamare move_to_this(), e copiare assegnazione e spostare l'assegnazione sono unificati in un unico singola funzione
  3. più efficiente di copia-e-swap, come swap coinvolge 3 assegnazioni, mentre qui solo 2, e questo codice non soffre i problemi menzionati in This Link

Ho ragione?

Grazie

Edit:

  1. Come @Leon sottolineato, forse è necessaria una funzione dedicata per liberare risorse, al fine di evitare la duplicazione del codice in move_to_this() e distruttore
  2. Come sottolineato @thorsan Per motivi di prestazioni estreme, è meglio separare DumbArray& operator=(DumbArray that) { move_to_this(that); return *this; } in DumbArray& operator=(const DumbArray &that) { DumbArray temp(that); move_to_this(temp); return *this; } (grazie a @MikeMB) e DumbArray& operator=(DumbArray &&that) { move_to_this(that); return *this; } per evitare un movimento extra operatoin

    Dopo l'aggiunta di un po 'di stampa di debug, ho scoperto che nessuna mossa in più è coinvolto in DumbArray& operator=(DumbArray that) {} quando si chiama come un incarico mossa

  3. Come @Erik Alapää ha sottolineato, è necessario un controllo auto-assegnazione prima delete in move_to_this()

+10

più adatto per [codereview.se]? – cpplearner

+1

Il codice non è eccezionalmente sicuro perché almeno il nuovo e l'eliminazione possono essere lanciati. – Resurrection

+1

@Resurrection, ciò che intendo per "eccezione sicura" non è che non genera eccezioni, ma che anche se viene lanciata un'eccezione, non infrange nulla e rende l'oggetto esistente non valido –

risposta

1

l'unico (piccolo) problema con il codice è la duplicazione di funzionalità tra move_to_this() e il distruttore, che è un problema di manutenzione dovrebbe essere necessario cambiare la vostra classe. Ovviamente può essere risolto estraendo quella parte in una funzione comune destroy().

mia critica dei "problemi" discussi da Scott Meyers nel suo blog:

Egli cerca di ottimizzare manualmente in cui il compilatore potrebbe fare un altrettanto buon lavoro se è abbastanza intelligente. La regola dei cinque può essere ridotto alla regola-di-quattro per

  • fornendo solo l'operatore di assegnazione copia che prende il suo argomento per valore e
  • senza preoccuparsi di scrivere l'operatore di assegnazione mossa (esattamente quello hai fatto).

Questo risolve automaticamente il problema delle risorse dell'oggetto a sinistra che vengono scambiate nell'oggetto a destra e che non vengono immediatamente rilasciate se l'oggetto di destra non è temporaneo.

Quindi, all'interno dell'implementazione dell'operatore di assegnazione copia secondo l'idioma copy-and-swap, swap() prenderà come uno dei suoi argomenti un oggetto in scadenza. Se il compilatore può allineare il distruttore di quest'ultimo, allora eliminerà definitivamente l'assegnazione del puntatore in più - anzi, perché salvare il puntatore che sarà delete sul passaggio successivo?

La mia conclusione è che è più semplice seguire l'idioma ben consolidato invece di complicare leggermente l'implementazione al fine di micro-ottimizzazioni che sono alla portata di un compilatore maturo.

+0

Per il primo punto, I non pensateci, potreste perdere la parte ': DumbArray()' del costruttore di move –

+0

@ChenRushan Sì, avete ragione. Corretto. – Leon

+0

Penso che la duplicazione possa essere risolta creando un'altra funzione dedicata alla liberazione di risorse che sono chiamate sia da 'move_to_this()' che da destructor –

8

commenti in linea, ma brevemente:

  • si desidera spostare tutte le assegnazioni e costruttori iniziativa possa offrire noexcept se possibile. La libreria standard è più più veloce se si attiva questa opzione, in quanto è possibile escludere qualsiasi gestione di eccezioni da algoritmi che riordinano una sequenza del proprio oggetto.

  • se si intende definire un distruttore personalizzato, renderlo no. Perché aprire la scatola di Pandora? Ho sbagliato su questo. È senza eccezioni per impostazione predefinita.

  • In questo caso, fornire la forte garanzia di eccezione è indolore e costa quasi nulla, quindi facciamolo.

codice:

#include <algorithm> 
#include <cstddef> 

class DumbArray { 
public: 
    DumbArray(std::size_t size = 0) 
    : size_(size), array_(size_ ? new int[size_]() : nullptr) { 
    } 

    DumbArray(const DumbArray& that) 
    : size_(that.size_), array_(size_ ? new int[size_] : nullptr) { 
     std::copy(that.array_, that.array_ + size_, array_); 
    } 

    // the move constructor becomes the heart of all move operations. 
    // note that it is noexcept - this means our object will behave well 
    // when contained by a std:: container 
    DumbArray(DumbArray&& that) noexcept 
    : size_(that.size_) 
    , array_(that.array_) 
    { 
     that.size_ = 0; 
     that.array_ = nullptr; 
    } 

    // noexcept, otherwise all kinds of nasty things can happen 
    ~DumbArray() // noexcept - this is implied. 
    { 
     delete [] array_; 
    } 

    // I see that you were doing by re-using the assignment operator 
    // for copy-assignment and move-assignment but unfortunately 
    // that was preventing us from making the move-assignment operator 
    // noexcept (see later) 
    DumbArray& operator=(const DumbArray& that) 
    { 
     // copy-swap idiom provides strong exception guarantee for no cost 
     DumbArray(that).swap(*this); 
     return *this; 
    } 

    // move-assignment is now noexcept (because move-constructor is noexcept 
    // and swap is noexcept) This makes vector manipulations of DumbArray 
    // many orders of magnitude faster than they would otherwise be 
    // (e.g. insert, partition, sort, etc) 
    DumbArray& operator=(DumbArray&& that) noexcept { 
     DumbArray(std::move(that)).swap(*this); 
     return *this; 
    } 


    // provide a noexcept swap. It's the heart of all move and copy ops 
    // and again, providing it helps std containers and algorithms 
    // to be efficient. Standard idioms exist because they work. 
    void swap(DumbArray& that) noexcept { 
     std::swap(size_, that.size_); 
     std::swap(array_, that.array_); 
    } 

private: 
    std::size_t size_; 
    int* array_; 
}; 

C'è un ulteriore miglioramento delle prestazioni si potrebbe fare nell'operatore mossa-assegnazione.

La soluzione che ho offerto fornisce la garanzia che un array spostato da sarà vuoto (con risorse deallocate). Questo potrebbe non essere quello che vuoi. Ad esempio, se si tiene traccia separatamente della capacità e della dimensione di un DumbArray (ad esempio, come std :: vector), è possibile che qualsiasi memoria allocata in venga conservata in that dopo lo spostamento. Ciò consentirebbe quindi di assegnare that mentre è possibile che si allontani senza un'altra allocazione di memoria.

Per attivare questa ottimizzazione, semplicemente implementare l'operatore sposta-assegnazione in termini di (noexcept) swap:

così da questa:

/// @pre that must be in a valid state 
    /// @post that is guaranteed to be empty() and not allocated() 
    /// 
    DumbArray& operator=(DumbArray&& that) noexcept { 
     DumbArray(std::move(that)).swap(*this); 
     return *this; 
    } 

a questo:

/// @pre that must be in a valid state 
    /// @post that will be in an undefined but valid state 
    DumbArray& operator=(DumbArray&& that) noexcept { 
     swap(that); 
     return *this; 
    } 

Nel caso del DumbArray, probabilmente vale la pena usare la forma più rilassata in pratica, ma attenzione ai bug sottili.

ad es.

DumbArray x = { .... }; 
do_something(std::move(x)); 

// here: we will get a segfault if we implement the fully destructive 
// variant. The optimised variant *may* not crash, it may just do 
// something_else with some previously-used data. 
// depending on your application, this may be a security risk 

something_else(x); 
+2

Ora i distruttori 'noexcept (true)' non sono predefiniti? –

+0

@ JDługosz solo se il valore predefinito IIRC –

+0

@RichardHodges [C++ 11 Working Draft N3376] (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3376.pdf) sembra non sono d'accordo con te (12.4 Destructors, item 3): 'Una dichiarazione di un distruttore che non ha una specifica di eccezione è implicitamente considerata come la specifica di eccezione come una dichiarazione implicita'. –

Problemi correlati