Cuando tantas cosas pueden salir mal, todo lo que hace es intentar, intentar, probar

Question

En serio, ¿cómo puede manejar todas esas excepciones sin volverse loco? ¿He leído demasiados artículos sobre el manejo de excepciones o qué? I intenté refactorizar esto un par de veces y cada vez parece que termino con algo aún peor. Tal vez debería admitir que las excepciones suceden y simplemente disfrutar de la codificación de la ruta feliz feliz? ;) Entonces, ¿qué hay de malo con este código (además del hecho de que era lo suficientemente flojo como para lanzar Exception en lugar de algo más específico)? Y, por supuesto, no seas fácil conmigo.

public void Export(Database dstDb) 
{ 
    try 
    { 
     using (DbConnection connection = dstDb.CreateConnection()) 
     { 
      connection.Open(); 
      DbTransaction transaction = connection.BeginTransaction(); 
      try 
      { 
       // Export all data here (insert into dstDb) 
       transaction.Commit(); 
      } 
      catch (SqlException sqlex) 
      { 
       ExceptionHelper.LogException(sqlex); 
       try 
       { 
        transaction.Rollback(); 
       } 
       catch (Exception rollbackEx) 
       { 
        logger.Error("An exception of type " + rollbackEx.GetType() + 
             " was encountered while attempting to roll back the transaction."); 
       } 
       throw new Exception("Error exporting message " + Type + " #" + Id + ": [" + sqlex.GetType() + "] " + sqlex.Message, sqlex); 
      } 
      catch (Exception ex) 
      { 
       try 
       { 
        transaction.Rollback(); 
       } 
       catch (Exception rollbackEx) 
       { 
        logger.Error("An exception of type " + rollbackEx.GetType() + 
             " was encountered while attempting to roll back the transaction."); 
       } 
       throw new Exception("Error exporting message " + Type + " #" + Id + ": [" + ex.GetType() + "] " + ex.Message, ex); 
      } 
     } 

     try 
     { 
      Status = MessageStatus.FINISHED; 
      srcDb.UpdateDataSet(drHeader.Table.DataSet, HEADERS, 
       CreateHeaderInsertCommand(), CreateHeaderUpdateCommand(), null); 
     } 
     catch (Exception statusEx) 
     { 
      logger.ErrorException("Failed to change message status to FINISHED: " + 
            Type + " #" + Id + ": " + statusEx.Message, statusEx); 
     } 
    } 
    catch (Exception importEx) 
    { 
     try 
     { 
      Status = MessageStatus.ERROR; 
      srcDb.UpdateDataSet(drHeader.Table.DataSet, HEADERS, 
        CreateHeaderInsertCommand(), CreateHeaderUpdateCommand(), null); 
     } 
     catch (Exception statusEx) 
     { 
      logger.ErrorException("Failed to change message status to ERROR: " + 
            Type + " #" + Id + ": " + statusEx.Message, statusEx); 
     } 
     AddErrorDescription(importEx.Message); 
     throw new Exception("Couldn't export message " + Type + " #" + Id + ", exception: " + importEx.Message, importEx); 
    } 
} 

Btw. Tantas veces intenté realmente ser lo más específico posible al formular preguntas; el resultado fue que no hubo visitas, ni respuestas, ni idea de cómo resolver el problema. Esta vez pensé en todas las ocasiones en las de otra persona pregunta me llamó la atención, supongo que esto era lo correcto a hacer :)

Actualización:

He intentado poner algunos de los consejos en práctica y esto es lo que se me ocurrió hasta ahora. Decidí modificar ligeramente el comportamiento: cuando no es posible establecer el estado del mensaje en FINISHED después de la exportación exitosa, lo considero como un trabajo que no se ha realizado del todo y restituyo y lanzo una excepción. Si todavía les queda algo de paciencia, háganme saber si es mejor. O arroja algunas críticas más a mí. Por cierto. Gracias por todas las respuestas, analicé cada una de ellas.

Lanzar una instancia de System.Exception no me pareció correcto, así que me deshice de eso, como se sugirió, y en su lugar decidí introducir una excepción personalizada. Esto, por cierto, tampoco parece correcto, ¿exagerado? Esto parece estar bien con los métodos públicos, pero un poco sobre-diseñado para un miembro privado, y aún así quiero saber que hubo un problema con el cambio de estado del mensaje en lugar de un problema con la conexión a la base de datos o algo así.

puedo ver dos formas de extraer los métodos aquí, pero todos ellos parecen mezclar responsabilidades que jgauffin mencionan en his comment: gestión de conexión de base de operaciones, manejo de bases de datos, lógica de negocio (datos de exportación). Diga el método ChangeStatus - es un cierto nivel de abstracción - cambia el estado de un mensaje y no le interesa cómo sucede esto, cómo persiste el mensaje, etc. Tal vez debería usar un patrón de Data Mapper para separar más responsabilidades , pero en este escenario aún bastante simple pensé que me saldría con Active Record. ¿Tal vez todo el diseño es tan enrevesado en este momento, que no veo dónde hacer los cortes?

public void Export(Database dstDb) 
{ 
    try 
    { 
     using (DbConnection connection = dstDb.CreateConnection()) 
     { 
      connection.Open(); 
      using (DbTransaction transaction = connection.BeginTransaction()) 
      { 
       // Export all data here (insert into dstDb) 
       ChangeStatus(MessageStatus.FINISHED); 
       transaction.Commit(); 
      } 
     } 
    } 
    catch (Exception exportEx) 
    { 
     try 
     { 
      ChangeStatus(MessageStatus.ERROR); 
      AddErrorDescription(exportEx.Message); 
     } 
     catch (Exception statusEx) 
     { 
      throw new MessageException("Couldn't export message and set its status to ERROR: " + 
        exportExt.Message + "; " + statusEx.Message, Type, Id, statusEx); 
     } 
     throw new MessageException("Couldn't export message, exception: " + exportEx.Message, Type, Id, exportEx); 
    } 
} 

private void ChangeStatus(MessageStatus status) 
{ 
    try 
    { 
     Status = status; 
     srcDb.UpdateDataSet(drHeader.Table.DataSet, HEADERS, 
      CreateHeaderInsertCommand(), CreateHeaderUpdateCommand(), null); 
    } 
    catch (Exception statusEx) 
    { 
     throw new MessageException("Failed to change message status to " + status + ":" + statusEx.Message, statusEx); 
    } 
} 

Answer 1

1. Los conjuntos de datos son la raíz de todo mal;) Trate de usar un ORM lugar.
2. Lee sobre single responsibility principle. tu código hace muchas cosas diferentes.
3. No capte excepciones solo para volver a lanzarlas
4. Use usando en la transacción y conexión.
5. No es necesario detectar todas las excepciones diferentes cuando todos los manejadores de excepciones hacen lo mismo. Los detalles de la excepción (tipo de excepción y propiedad del mensaje) proporcionarán información.

Answer 2

El manejo de excepciones es un tema muy discutido y se implementa de muy diversas formas.Hay algunas reglas que trato de cumplir al manejar excepciones:

1. Nunca arroje System.Exception, generalmente hay suficientes tipos de excepciones para llenar su requerimiento, si no, especialícese. Ver: http://www.fidelitydesign.net/?p=45
2. Solo arroje una excepción si el método en sí no puede hacer otra cosa que lanzar una excepción. Si un método puede recuperar/manejar las variaciones esperadas de entrada/comportamiento, entonces no arroje una excepción. Lanzar excepciones es un proceso que consume muchos recursos.
3. Nunca atrape una excepción solo para volver a lanzarlo. Capture y vuelva a lanzar si necesita realizar algún trabajo adicional, como informar la excepción o envolver la excepción en otra excepción (normalmente hago esto para el trabajo de WCF o trabajo transaccional).

Estas son sólo las cosas que yo personalmente, una gran cantidad de desarrolladores que prefieren hacer formas en las que se sienten cómodos ....

Answer 3

Adicionalmente a la great answer of jgauffin.

6. no capte excepciones solo para registrarlas. atraparlos en el nivel más alto y registrar todas las excepciones allí.

Editar:

Dado que el registro excepción en todo el lugar tiene al menos estas desventajas: varias veces

1. La misma excepción se puede registrar, que llena el registro de forma innecesaria. Es difícil decir cuántos errores realmente ocurrieron.
2. Si la persona que llama capta y maneja la excepción, el llamador aún tiene mensajes de error en el archivo de registro, lo que al menos es confuso.

Answer 4

Cree una clase de registro que maneje retrocesos para sus propias fallas (es decir, intenta SQL, si eso falla escribe en el registro de eventos, si eso falla, escribe en un archivo de registro local, etc.).

Además, no recomiendo que detecte un error y arroje uno diferente. Cada vez que lo hace, está perdiendo valiosa información de rastreo/depuración sobre la fuente de excepción original.

public void Export(Database dstDb) 
{ 
    try 
    { 
    using (DbConnection connection = dstDb.CreateConnection()) 
    { 
     connection.Open(); 
     using (DbTransaction transaction = connection.BeginTransaction()) 
     { 
      // Export all data here (insert into dstDb) 
      ChangeStatus(MessageStatus.FINISHED); 
      transaction.Commit(); 
     } 
    } 
    } 
    catch (Exception exportEx) 
    { 
    LogError(exportEx);// create a log class for cross-cutting concerns 
     ChangeStatus(MessageStatus.ERROR); 
     AddErrorDescription(exportEx.Message); 

    throw; // throw preserves original call stack for debugging/logging 
    } 
} 

private void ChangeStatus(MessageStatus status) 
{ 
    try 
    { 
    Status = status; 
    srcDb.UpdateDataSet(drHeader.Table.DataSet, HEADERS, 
     CreateHeaderInsertCommand(), CreateHeaderUpdateCommand(), null); 
    } 
    catch (Exception statusEx) 
    { 
    Log.Error(statusEx); 
    throw; 
    } 
} 

También para cualquier situación donde se siente que hay bloques try/catch adicionales necesarios, hacer que su propio método, si son demasiado feo. Me gusta la respuesta de Stefan Steinegger, donde la llamada de nivel superior en su aplicación es el mejor lugar para atrapar.

Parte del problema que aquí me imagino es que algo es mutable que hace que intentes establecer el estado después de un error. Si puede factorizar su objeto para que esté en un estado constante independientemente de si su transacción funciona o no, no tiene que preocuparse por esa parte.