2012-07-05 10 views
29

Estoy familiarizado con las ventajas de RAII, pero recientemente he tropezado con un problema en el código como el siguiente:Cómo manejar el fracaso constructor para RAII

class Foo 
{ 
    public: 
    Foo() 
    { 
    DoSomething(); 
    ...  
    } 

    ~Foo() 
    { 
    UndoSomething(); 
    } 
} 

Todo bien, excepto que el código en la sección constructor ... lanzó una excepción, con el resultado de que UndoSomething() nunca recibió una llamada.

Hay maneras obvias de la fijación de ese tema en particular, como el abrigo ... en un bloque try/catch que luego llama UndoSomething(), sino una: eso es la duplicación de código, y b: bloques try/catch son un olor código que lo intento y evitar usando técnicas RAII. Y, es probable que el código empeore y sea más propenso a errores si hay varios pares de Do/Deshacer involucrados, y tenemos que limpiar a mitad de camino.

Me pregunto si hay un mejor enfoque para hacer esto: ¿tal vez un objeto separado toma un puntero de función e invoca la función cuando, a su vez, se destruye?

class Bar 
{ 
    FuncPtr f; 
    Bar() : f(NULL) 
    { 
    } 

    ~Bar() 
    { 
    if (f != NULL) 
     f(); 
    } 
} 

Sé que no se compilará pero debe mostrar el principio. Foo luego se convierte en ...

class Foo 
{ 
    Bar b; 

    Foo() 
    { 
    DoSomething(); 
    b.f = UndoSomething; 
    ...  
    } 
} 

Tenga en cuenta que foo ahora no requiere un destructor. ¿Eso suena como más problemas de los que vale la pena, o es este un patrón común con algo útil en el impulso para manejar el trabajo pesado para mí?

+3

try/catch is _not_ code smell, y es a menudo subutilizado IMO. –

+2

mira aquí: http://www.parashift.com/c++faq-lite/selfcleaning-members.html – MadScientist

+2

@MooingDuck: De hecho, por sí mismos no huelen. Pero 'try {} catch (...) {throw;} 'tiene un fuerte olor. –

Respuesta

30

El problema es que su clase está tratando de hacer demasiado. El principio de RAII es que adquiere un recurso (ya sea en el constructor o posterior) y el destructor lo libera; la clase existe únicamente para administrar ese recurso.

En su caso, cualquier cosa que no sea DoSomething() y UndoSomething() debe ser responsabilidad del usuario de la clase, no de la clase en sí.

Como dice Steve Jessop en los comentarios: si tiene varios recursos para adquirir, cada uno debe ser administrado por su propio objeto RAII; y podría tener sentido agregar estos como miembros de datos de otra clase que construye cada uno a su vez. Luego, si falla cualquier adquisición, todos los recursos previamente adquiridos serán liberados automáticamente por los destructores de los miembros individuales de la clase.

(Además, recuerde el Rule of Three; su clase necesita evitar la copia o implementarla de una manera sensata para evitar llamadas múltiples al UndoSomething()).

+5

Lo que iba a decir. Añadiría que una vez que haya escrito una clase para administrar cada recurso, puede agregar varios de ellos como miembros de datos de otra clase, si la combinación de recursos tiene sentido. Si se lanza un constructor miembro de datos, se destruyen todos los miembros ya inicializados. –

+0

Sí, pero la adquisición del recurso implica varios pasos: 'DoSomething' es el primer paso para adquirir el recurso. – Roddy

+5

@Roddy: ITYM, "hay varios recursos para adquirir". Puede que aún no se dé cuenta de que son recursos separados, pero el patrón RAII está haciendo todo lo posible para decirle :-) –

6

que resolvería esto utilizando RAII, también:

class Doer 
{ 
    Doer() 
    { DoSomething(); } 
    ~Doer() 
    { UndoSomething(); } 
}; 
class Foo 
{ 
    Doer doer; 
public: 
    Foo() 
    { 
    ... 
    } 
}; 

El hacedor se crea antes de que comience el cuerpo ctor y se destruye o bien cuando el destructor falla a través de una excepción o cuando el objeto se destruye con normalidad.

17

Simplemente haga DoSomething/UndoSomething en un mango adecuado RAII:

struct SomethingHandle 
{ 
    SomethingHandle() 
    { 
    DoSomething(); 
    // nothing else. Now the constructor is exception safe 
    } 

    SomethingHandle(SomethingHandle const&) = delete; // rule of three 

    ~SomethingHandle() 
    { 
    UndoSomething(); 
    } 
} 


class Foo 
{ 
    SomethingHandle something; 
    public: 
    Foo() : something() { // all for free 
     // rest of the code 
    } 
} 
+0

¿Qué es '= borrar'? – Nick

+2

@Nick hace que la compilación falle si esa función se selecciona mediante resolución de sobrecarga. Es una nueva característica. Puede lograr algo similar en los compiladores que no admiten esta característica haciéndola privada. –

6

que tienes demasiado en su una clase.Mover HacerAlgo/UndoSomething a otra clase ('Algo'), y tienen un objeto de esa clase como parte de la clase Foo, de esta manera:

class Foo 
{ 
    public: 
    Foo() 
    { 
    ...  
    } 

    ~Foo() 
    { 
    } 

    private: 
    class Something { 
    Something() { DoSomething(); } 
    ~Something() { UndoSomething(); } 
    }; 
    Something s; 
} 

Ahora, HacerAlgo ha sido llamado por el momento el constructor de Foo se llama, y si el constructor de Foo lanza, entonces UndoSomething se llama correctamente.

6

try/catch es not code smell en general, se debe utilizar para manejar los errores. En su caso, sin embargo, sería un olor codificado, porque no está manejando un error, simplemente limpiando. Para eso están los destructores.

(1) Si todo lo en el destructor debe ser llamada cuando el constructor falla, simplemente moverlo a una función privada de limpieza, que es llamada por el destructor, y el constructor en el caso de fallo. Esto parece ser lo que ya has hecho. Buen trabajo.

(2) Una mejor idea es: si hay múltiples pares de deshacer/deshacer que pueden destruir por separado, deben estar envueltos en su propia pequeña clase RAII, que hace su minitarea, y se limpia solo. No me gusta tu idea actual de darle una función de puntero de limpieza opcional, eso es simplemente confuso. La limpieza siempre debe emparejarse con la inicialización, ese es el concepto central de RAII.

+0

Gracias. De acuerdo re. excepciones, pero he visto tanto código de limpieza en las declaraciones de captura que todavía los trato con sospecha. El concepto era que la función de limpieza es 'opcional' para evitar llamar a UndoSomething si se lanza una excepción antes de llamar a DoSomething. 'void Foo(): b (UndoSomething) {... DoSomething());' – Roddy

+0

@Roddy: si la limpieza está en el miniclass, la inicialización también debe estar en el miniclass, por lo que no es un problema. –

+0

suena como 'void Foo(): b (DoSomething, UndoSomething)' es obligatorio. Interesante ... – Roddy

0

Reglas de oro:

  • Si la clase está gestionando manualmente la creación y eliminación de algo, se está haciendo demasiado.
  • Si la clase ha escrito manualmente copia a la cesión/-Construcción, es probable que sea demasiado gestionando
  • La excepción a esto: Una clase que tiene la única finalidad de gestionar exactamente una entidad

Ejemplos para la la tercera regla es std::shared_ptr, std::unique_ptr, scope_guard, std::vector<>, std::list<>, scoped_lock, y por supuesto la clase Trasher a continuación.


Addendum.

usted podría ir tan lejos y escribir algo para interactuar con la materia de estilo C:

#include <functional> 
#include <iostream> 
#include <stdexcept> 


class Trasher { 
public: 
    Trasher (std::function<void()> init, std::function<void()> deleter) 
    : deleter_(deleter) 
    { 
     init(); 
    } 

    ~Trasher() 
    { 
     deleter_(); 
    } 

    // non-copyable 
    Trasher& operator= (Trasher const&) = delete; 
    Trasher (Trasher const&) = delete; 

private: 
    std::function<void()> deleter_; 
}; 

class Foo { 
public: 
    Foo() 
    : meh_([](){std::cout << "hello!" << std::endl;}, 
      [](){std::cout << "bye!" << std::endl;}) 
    , moo_([](){std::cout << "be or not" << std::endl;}, 
      [](){std::cout << "is the question" << std::endl;}) 
    { 
     std::cout << "Fooborn." << std::endl; 
     throw std::runtime_error("oh oh"); 
    } 

    ~Foo() { 
     std::cout << "Foo in agony." << std::endl; 
    } 

private: 
    Trasher meh_, moo_; 
}; 

int main() { 
    try { 
     Foo foo; 
    } catch(std::exception &e) { 
     std::cerr << "error:" << e.what() << std::endl; 
    } 
} 

de salida:

hello! 
be or not 
Fooborn. 
is the question 
bye! 
error:oh oh 

Así, ~Foo() nunca se acaba, pero su init/borrar par se encuentra.

Una cosa buena es: si su función init se lanza, no se invocará su función de eliminación, ya que cualquier excepción lanzada por la función init pasa directamente a Trasher() y por lo tanto ~Trasher() no se ejecutará.

Nota: Es importante que exista un try/catch más externo, de lo contrario, la norma no exige el desenrollado de la pila.