2011-12-15 14 views
8

He leído Necesito implementar IDisposable si mi clase tiene una variable miembro que a su vez es IDisposable. Bueno, yo estoy poniendo en práctica IDisposable interfaz porque mi clase contiene un objeto de base de datos (db miembro de la clase más adelante), que es en sí IDisposable:Tratando con excepciones en el constructor al implementar IDisposable

public class CommissionModel : IDisposable 
    { 
     protected PetaPoco.Database db; 

     public CommissionModel() 
     { 
      string connectionString = "Server=localhost;..."; 

      // The line below may throw an exception (!!!) 
      db = new PetaPoco.Database(connectionString, "mysql");    
     } 

     // Automatically close database connection 
     public void Dispose() 
     { 
      if (db != null) 
       db.Dispose(); 

      db = null; 
     } 

     public void InsertRecord(Record somerecord) 
     { 
      // ... 
      db.Insert(somerecord); 
     } 

El problema es creación de instancias de db miembro puede fallar en algunas circunstancias. ¿Qué debo hacer cuando se lanza la excepción en el constructor y no se crea el objeto de la base de datos? ¿Debo volver a lanzar la excepción o tal vez verificar si db != null en el método InsertRecord?

+0

IMO debe volver a lanzar la excepción. Puede envolver la excepción original en un nivel superior uno para tener un mejor aislamiento de capa. Es mejor informar que algo va mal. – Krzysztof

+0

Eche un vistazo a [SOLID] (https://en.wikipedia.org/wiki/Solid_%28object-oriented_design%29), específicamente en [SRP] (https://en.wikipedia.org/wiki/Single_responsibility_principle) y [DI] (https://en.wikipedia.org/wiki/Dependency_inversion_principle) – oleksii

Respuesta

1

Solo debe detectar las excepciones que puede manejar. Desde el código, parece que la inicialización de la variable db puede mostrar que hay un problema al comunicarse con su servidor de base de datos.

Mi sugerencia es dejar el código tal como está y manejar las excepciones en una ubicación central como el evento Application_Error en Global.asax o el evento que está inicializando CommissionModel.

2

Idealmente, no debería estar haciendo ningún "trabajo" en el constructor. El trabajo en el constructor realmente debería vincular referencias de objetos. Esto le permitiría probar esta clase en una unidad conectando clases simuladas en su lugar.

Tal vez puedas probar:

public CommissionModel(PetaPoco.Database db) { 
    this.db = db; 
} 
+1

+1 Esta es la mejor respuesta que apunta OP hacia un mejor diseño. También mencionaría los principios SOLID y quizás use 'IDatabase' en lugar de' PetaPoco.Database' – oleksii

+0

@oleksii, no estoy de acuerdo con usted en eso. La razón detrás de la implementación de 'CommissionModel' es aislar toda la manipulación de datos de bajo nivel y la funcionalidad de manejo de bases de datos de otras partes del código. Además, un día puedo decidir cambiar a otro marco ORM en lugar de PetaPoco. – ezpresso

+0

@ezpresso * "[...] un día puedo decidir cambiar a otro marco ORM en lugar de PetaPoco" * es exactamente por eso es posible que deba pasar una interfaz, que ahora se implementará con PetaPoco, y luego, debería Si lo desea, puede cambiar a otro ORM. No estoy seguro de seguirte en cierto sentido con qué cosa particular no estás de acuerdo. ¿Podrías por favor explicármelo? – oleksii

3

Si el constructor lanza una excepción, el consumidor de su código nunca recibirá una referencia a una instancia de su clase. La memoria de clase parcialmente inicializada se recopilará finalmente y no se invocará Dispose.

En general, recomendaría mover una operación que puede fallar debido a circunstancias externas (y no debido a un uso erróneo, como un valor de parámetro no válido) a un método diferente, como "Conectar".

2

Parece que no tiene que hacer nada.

Si se interrumpe el proceso de construcción, el recurso no se crea. En la mayoría de los escenarios de uso (esto queda fuera de su clase), el Dispose() nunca será llamado. Pero incluso cuando lo es, su código if (db != null) es suficiente protección.

Unos pocos puntos de menor importancia:

  • la db = null; dentro Dispose() no tiene sentido.
  • InsertRecord() debe comenzar por comprobar if (db != null)
1

Si no hay manera de que pueda producirse una excepción (*) entre el momento en el constructor adquiere un recurso y el tiempo que se devuelve a la aplicación, no hay nada que preocuparse acerca de.

Si es posible que una excepción podría ocurrir en tales circunstancias, sugeriría un patrón como:

 
public class CommissionModel : IDisposable 
    { 
     protected PetaPoco.Database db; 
     protected OtherResourceType resource2; 

     public CommissionModel() 
     { 
      Boolean ok; 
      string connectionString = "Server=localhost;..."; 

      ok = false; 
      try 
      { 
       // Either of the next two lines may throw an exception 
       db = new PetaPoco.Database(connectionString, "mysql"); 
       resource2 = new OtherResourceType(); 
       // Once we make it this far, we should be successful 
       ok = true; 
      } 
      finally 
      { 
       if (!ok) 
        Dispose(); 
      } 
     } 

     // Automatically close database connection 
     public void Dispose() 
     { 
      Zap(ref db); // Method to Dispose and null out, only if not null 
      Zap(ref resource2); 
     } 
    } 

(*) Casi siempre es posible que un ThreadAbortException a ocurrir si alguna quiere decir ogro desagradable llama la banda de rodamiento. Aborte su código, pero en realidad no hay nada que hacer al respecto en cualquier caso.

Cuestiones relacionadas