Cómo defensiva debo programa?[cerrado]

StackOverflow https://stackoverflow.com/questions/1053215

Pregunta

yo estaba trabajando con una pequeña rutina que se utiliza para crear una conexión de base de datos:

Antes de

public DbConnection GetConnection(String connectionName)
{
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
   DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);
   DbConnection conn = factory.CreateConnection();
   conn.ConnectionString = cs.ConnectionString;
   conn.Open();

   return conn;
}

Entonces comencé a buscar en el .NET framework documentación, a ver lo que la documentado el comportamiento de las distintas son las cosas, y ver si puedo manejar.

Por ejemplo:

ConfigurationManager.ConnectionStrings...

El documentación dice que llamar a ConnectionStrings lanza un ConfigurationErrorException si no puede recuperar la colección.En este caso no hay nada que yo pueda hacer para controlar esta excepción, así que voy a dejarlo ir.

Siguiente parte es el de la indexación de la ConnectionStrings para encontrar connectionName:

...ConnectionStrings[connectionName];

En este ejemplo, el ConnectionStrings documentación dice que la propiedad devolverá null si el nombre de la conexión no se pudo encontrar.puedo comprobar esta sucediendo, y lanzar una excepción para permitir que alguien de alto, hasta que se dio un inválido connectionName:

ConnectionStringSettings cs= 
      ConfigurationManager.ConnectionStrings[connectionName];
if (cs == null)
   throw new ArgumentException("Could not find connection string \""+connectionName+"\"");

me repita el mismo ejercicio con:

DbProviderFactory factory = 
      DbProviderFactories.GetFactory(cs.ProviderName);

El GetFactory el método no tiene la documentación en ¿qué pasa si una fábrica para la especificada ProviderName no se pudo encontrar.No documentados para volver null, pero puedo estar a la defensiva, y verificación null:

DbProviderFactory factory = 
      DbProviderFactories.GetFactory(cs.ProviderName);
if (factory == null) 
   throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");

Siguiente es la construcción de la DbConnection objeto:

DbConnection conn = factory.CreateConnection()

De nuevo el documentación no dice qué pasa si se podía crear una conexión, pero de nuevo me puede comprobar por un valor null objeto de devolución:

DbConnection conn = factory.CreateConnection()
if (conn == null) 
   throw new Exception.Create("Connection factory did not return a connection object");

Siguiente es la configuración de una propiedad del objeto de Conexión:

conn.ConnectionString = cs.ConnectionString;

Los documentos no dicen lo que sucede si no puede establecer la cadena de conexión.Qué es una excepción?Hace ignorarlo?Como con la mayoría de excepción, si hubo un error al intentar establecer la cadena de conexión de una conexión, no hay nada que yo pueda hacer para recuperarse de ella.Así que voy a hacer nada.

Y por último, la apertura de la conexión de base de datos:

conn.Open();

El Método abierto de DbConnection es abstracta, por lo que toca a cualquier proveedor que es descendiente de DbConnection para decidir qué excepciones que lanzan.También no hay ninguna orientación en el resumen de métodos de Abrir documentación acerca de lo que puede ocurrir si hay un error.Si hubo un error de conexión, yo sé que no puedo manejarlo - voy a tener que dejar que la burbuja hasta que la persona que llama puede mostrar una interfaz de usuario para el usuario, y vamos a tratar de nuevo.

Después de

public DbConnection GetConnection(String connectionName)
{
   //Get the connection string info from web.config
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];

   //documented to return null if it couldn't be found
    if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");

   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);

   //Undefined behaviour if GetFactory couldn't find a provider.
   //Defensive test for null factory anyway
   if (factory == null)
      throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");

   //Have the factory give us the right connection object
   DbConnection conn = factory.CreateConnection();

   //Undefined behaviour if CreateConnection failed
   //Defensive test for null connection anyway
   if (conn == null)
      throw new Exception("Could not obtain connection from factory");

   //Knowing the connection string, open the connection
   conn.ConnectionString = cs.ConnectionString;
   conn.Open()

   return conn;
}

Resumen

Así que mis cuatro de la línea de la función, se convirtió en 12 líneas, y requiere 5 minutos de la documentación de búsqueda.En el final me hizo coger un caso en el que un método está permitido devolver null.Pero en la práctica todo lo que hice fue convertir una excepción de infracción de acceso (si se me intentos para llamar a métodos en una referencia nula) en un InvalidArgumentException.

yo también la captura de dos posibles casos donde no podría ser null devolución de objetos;pero de nuevo, yo sólo se comercializan una excepción para otro.

En el lado positivo, se hizo la captura de dos de los problemas, y explicar lo que pasó en el mensaje de excepción, en lugar de las cosas malas que suceden en el camino (es decir,la pelota se detiene aquí)

Pero ¿vale la pena?Es esto una exageración?Es esta la defensiva de programación ido mal?

¿Fue útil?

Solución

Verificar manualmente una configuración y lanzar una excepción no es mejor que simplemente dejar que el marco lance la excepción si falta la configuración. Solo está duplicando las comprobaciones de precondición que ocurren dentro de los métodos de marco de todos modos, y hace que el código sea detallado sin ningún beneficio. (En realidad, podría estar eliminando información arrojando todo como la clase de excepción base. Las excepciones generadas por el marco suelen ser más específicas).

Editar: esta respuesta parece ser algo controvertida, por lo que un poco de elaboración: programación defensiva significa & "; prepárate para lo inesperado &"; (o " se paranoico ") y una de las formas de hacerlo es hacer muchas comprobaciones previas. En muchos casos, esta es una buena práctica, sin embargo, como con todas las prácticas, el costo debe compararse con los beneficios.

Por ejemplo, no proporciona ningún beneficio arrojar un & "; No se pudo obtener la conexión de fábrica &"; excepción, ya que no dice nada sobre por qué no se pudo obtener el proveedor, y la siguiente línea arrojará una excepción de todos modos si el proveedor es nulo. Por lo tanto, el costo de la verificación de precondición (en tiempo de desarrollo y complejidad del código) no está justificado.

Por otro lado, la verificación para verificar que la configuración de la cadena de conexión está presente puede estar justificada, ya que la excepción puede ayudar a decirle al desarrollador cómo resolver el problema. La excepción nula que obtendrá en la siguiente línea de todos modos no indica el nombre de la cadena de conexión que falta, por lo que su verificación de precondición proporciona algún valor. Si su código es parte de un componente, por ejemplo, el valor es bastante grande, ya que el usuario del componente podría no saber qué configuraciones requiere el componente.

Una interpretación diferente de la programación defensiva es que no solo debe detectar condiciones de error, también debe intentar recuperarse de cualquier error o excepción que pueda ocurrir. No creo que sea una buena idea en general.

Básicamente, solo debe manejar las excepciones sobre las que puede hacer algo. Las excepciones de las que no se puede recuperar de todos modos, solo se deben pasar al controlador de nivel superior. En una aplicación web, el controlador de nivel superior probablemente solo muestra una página de error genérico. Pero no hay mucho que hacer en la mayoría de los casos, si la base de datos está fuera de línea o falta alguna configuración crucial.

Algunos casos donde ese tipo de programación defensiva tiene sentido, es si acepta la entrada del usuario, y esa entrada puede conducir a errores. Si, por ejemplo, el usuario proporciona una URL como entrada y la aplicación intenta obtener algo de esa URL, entonces es muy importante que verifique que la URL se vea correcta y maneje cualquier excepción que pueda resultar de la solicitud. Esto le permite proporcionar comentarios valiosos al usuario.

Otros consejos

Bueno, depende de quién sea tu audiencia.

Si está escribiendo código de biblioteca que espera que muchas otras personas usen, que no le hablarán sobre cómo usarlo, entonces no es excesivo. Apreciarán tu esfuerzo.

(Dicho esto, si está haciendo eso, le sugiero que defina mejores excepciones que solo System.Exception, para que sea más fácil para las personas que desean detectar algunas de sus excepciones pero no otras).

Pero si solo lo va a usar usted mismo (o usted y su amigo), entonces obviamente es excesivo, y probablemente lo lastime al final al hacer que su código sea menos legible.

Desearía poder hacer que mi equipo codifique de esta manera. La mayoría de la gente ni siquiera entiende la programación defensiva. ¡Lo mejor que hacen es envolver todo el método en una declaración try catch y dejar que todas las excepciones sean manejadas por el bloque genérico de excepciones!

Felicitaciones a ti Ian. Puedo entender tu dilema. He pasado por lo mismo yo mismo. Pero lo que hiciste probablemente ayudó a algunos desarrolladores durante varias horas de golpe de teclado.

Recuerde, cuando usa una API de .NET Framework, ¿qué espera de ella? ¿Qué parece natural? Haz lo mismo con tu código.

Sé que lleva tiempo. Pero entonces la calidad tiene un costo.

PD: Realmente no tienes que manejar todos los errores y lanzar una excepción personalizada. Recuerde que su método solo será utilizado por otros desarrolladores. Deberían ser capaces de descubrir las excepciones del marco común por sí mismos. ESO no vale la pena.

Tu " antes de " ejemplo tiene la distinción de ser claro y conciso.

Si algo está mal, el marco eventualmente lanzará una excepción. Si no puede hacer nada con respecto a la excepción, también puede dejar que se propague por la pila de llamadas.

Sin embargo, hay ocasiones en que se lanza una excepción en el interior del marco que realmente no arroja luz sobre cuál es el problema real. Si su problema es que no tiene una cadena de conexión válida, pero el marco genera una excepción como & Quot; uso no válido de nulo, & Quot; entonces a veces es mejor detectar la excepción y volver a lanzarla con un mensaje que sea más significativo.

Verifico mucho los objetos nulos, ya que necesito un objeto real para operar, y si el objeto está vacío, la excepción que se lanza será oblicua, por decir lo menos. Pero solo verifico si hay objetos nulos si sé que eso es lo que ocurrirá. Algunas fábricas de objetos no devuelven objetos nulos; en su lugar, lanzan una excepción, y la comprobación de nulo será inútil en estos casos.

No creo que me gustaría escribir alguna de que null-comprobación de referencias de la lógica - al menos, no de la manera que lo he hecho.

Mis programas que obtenga los ajustes de configuración el archivo de configuración de aplicación de comprobar todos los ajustes en el inicio.Yo generalmente construir una clase estática que contiene la configuración de referencia y que clase de propiedades (y no el ConfigurationManager) en otras partes de la aplicación.Hay dos razones para esto.

En primer lugar, si la aplicación no está configurado correctamente, no va a funcionar.Prefiero saber eso en el momento en que el programa lee el archivo de configuración que en algún momento en el futuro, al intentar crear una conexión de base de datos.

Segundo, la comprobación de la validez de configuración no debe ser la preocupación de los objetos que se basan en la configuración.No hay sentido en lo mismo insertar controles en todas partes en todo el código, si usted ya haya realizado las comprobaciones hasta el frente.(Hay excepciones a esto, sin duda - por ejemplo, las aplicaciones de ejecución donde usted necesita para ser capaz de cambiar la configuración mientras se ejecuta el programa y los cambios reflejados en el comportamiento del programa;en programas como este, tendrás que ir a la ConfigurationManager siempre que necesite un ajuste.)

Yo no haría el null-comprobación de referencias en el GetFactory y CreateConnection llamadas, tampoco.¿Cómo escribir un caso de prueba para el ejercicio de ese código?Usted no puede, porque no sabe cómo hacer que los métodos devuelven null - ni siquiera sabes que es posible para hacer esos métodos devuelven null.Así que he sustituido uno de los problemas - el programa puede lanzar una NullReferenceException bajo las condiciones que usted no entiende - con el otro, más significativo:en virtud de estos mismos misterioso condiciones, el programa ejecutará el código que no he probado.

Mi regla de oro es:

  

no atrapar si el mensaje de la   excepción lanzada es relevante para el   llamante.

Por lo tanto, NullReferenceException no tiene un mensaje relevante, comprobaría si es nulo y lanzaría una excepción con un mensaje mejor. ConfigurationErrorException es relevante, así que no lo atrapo.

La única excepción es si " contract " de GetConnection no recupera la cadena de conexión necesariamente en un archivo de configuración.

Si es el caso, GetConnection DEBE tener un contrato con una excepción personalizada que indique que no se pudo recuperar la conexión, entonces puede ajustar ConfigurationErrorException en su excepción personalizada.

La otra solución es especificar que GetConnection no puede lanzar (pero puede devolver nulo), luego agrega un " ExceptionHandler " a tu clase.

Falta la documentación de su método. ;-)

Cada método tiene algunos parámetros de entrada y salida definidos y un comportamiento resultante definido. En su caso, algo como: & Quot; Devuelve una conexión abierta válida en caso de éxito, de lo contrario devuelve nulo (o lanza una XXXException, como lo desee). Teniendo en cuenta este comportamiento, ahora puede decidir qué tan defensivo debe programar.

  • Si su método debe exponer información detallada sobre por qué y qué falló, hágalo como lo hizo y verifique y capture todo y todo y devuelva la información apropiada.

  • Pero, si solo está interesado en una DBConnection abierta o simplemente null (o alguna excepción definida por el usuario) en caso de falla, simplemente envuelva todo en un try / catch y devuelva nulo (o alguna excepción) en caso de error, y el objeto más.

Entonces diría que depende del comportamiento del método y del resultado esperado.

En general, las excepciones específicas de la base de datos deben capturarse y volverse a lanzar como algo más general, como la excepción (hipotética) DataAccessFailure. El código de nivel superior en la mayoría de los casos no necesita saber que está leyendo los datos de la base de datos.

Otra razón para atrapar estos errores rápidamente es que a menudo incluyen algunos detalles de la base de datos en sus mensajes, como " No existe esa tabla: ACCOUNTS_BLOCKED " o " Clave de usuario no válida: 234234 " ;. Si esto se propaga al usuario final, es malo de varias maneras:

  1. Confuso.
  2. Posible violación de la seguridad.
  3. Vergonzoso para la imagen de su empresa (imagine un cliente leyendo un mensaje de error con gramática cruda).

Lo hubiera codificado exactamente como tu primer intento.

Sin embargo, el usuario de esa función protegería el objeto de conexión con un bloque USING.

Realmente no me gusta traducir excepciones como lo hacen sus otras versiones, ya que hace que sea muy difícil averiguar por qué se rompió (¿Base de datos inactiva? ¿No tiene permiso para leer el archivo de configuración, etc.?).

La versión modificada, no añade mucho valor, siempre y cuando la aplicación tiene una AppDomain.UnexpectedException controlador que vuelca el exception.InnerException de la cadena y todos los seguimientos de pila a un archivo de registro de algún tipo (o mejor aún, una captura de minivolcado) y, a continuación, llamar a Environment.FailFast.

A partir de esa información, será razonablemente sencillo para determinar qué salió mal, sin necesidad de complicar el código del método adicional de comprobación de error.

Tenga en cuenta que es preferible manejar AppDomain.UnexpectedException y llame a Environment.FailFast en lugar de tener un nivel superior try/catch (Exception x) porque con la última, la causa original del problema probablemente será oscurecido por otras excepciones.

Esto es debido a que si se captura una excepción, cualquier abierto finally los bloques se va a ejecutar, y probablemente va a tirar más excepciones que se va a ocultar la excepción original (o, peor aún, se eliminarán los archivos en un intento de revertir algunas de estado, posiblemente los archivos equivocados, tal vez incluso archivos importantes).Nunca se debe capturar una excepción que indica que un programa no válido estado que no sabe cómo manejar, incluso en un nivel superior main la función try/catch el bloque.Manejo de AppDomain.UnexpectedException y llamando Environment.FailFast es una diferente (y deseable) hervidor de pescado, porque no deja de finally los bloques de ejecución, y si usted está tratando de detener su programa y registro de información útil sin hacer más daño, definitivamente no desea ejecutar su finally los bloques.

No te olvides de buscar OutOfMemoryExceptions ... ya sabes, podría suceder.

Los cambios de Iain me parecen sensatos.

Si estoy usando un sistema y lo uso incorrectamente, quiero la máxima información sobre el mal uso. P.ej. si olvido insertar algunos valores en una configuración antes de llamar a un método, quiero una InvalidOperationException con un mensaje que detalle mi error, no una KeyNotFoundException / NullReferenceException.

Se trata de contexto IMO. He visto algunos mensajes de excepción bastante impenetrables en mi tiempo, pero otras veces la excepción predeterminada que viene del marco está perfectamente bien.

En general, creo que es mejor equivocarse con precaución, especialmente cuando está escribiendo algo que otras personas usan mucho o que normalmente se encuentra en lo profundo del gráfico de llamadas, donde el error es más difícil de diagnosticar.

Siempre trato de recordar que, como desarrollador de un código o sistema, estoy en una mejor posición para diagnosticar fallas que alguien que solo lo está usando. A veces ocurre que unas pocas líneas de código de verificación + un mensaje de excepción personalizado pueden ahorrar horas de tiempo de depuración de forma acumulativa (y también hacer que su propia vida sea más fácil, ya que no se detiene en la máquina de otra persona para depurar su problema).

En mis ojos, tu " después de " La muestra no es realmente defensiva. Porque defensivo sería verificar las partes bajo su control, que sería el argumento connectionName (verificar nulo o vacío, y lanzar una excepción ArgumentNullException).

¿Por qué no dividir el método que tiene después de agregar toda la programación defensiva? Tiene un montón de bloques lógicos distintos que garantizan métodos separados. ¿Por qué? Porque entonces encapsulas la lógica que pertenece, y tu método público resultante simplemente conecta esos bloques de la manera correcta.

Algo como esto (editado en el editor SO, por lo que no se comprueba la sintaxis / compilador. Puede que no se compile ;-)) 

private string GetConnectionString(String connectionName)
{

   //Get the connection string info from web.config
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];

   //documented to return null if it couldn't be found
   if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
   return cs;
}

private DbProviderFactory GetFactory(String ProviderName)
{
   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = DbProviderFactories.GetFactory(ProviderName);

   //Undefined behaviour if GetFactory couldn't find a provider.
   //Defensive test for null factory anyway
   if (factory == null)
      throw new Exception("Could not obtain factory for provider \""+ProviderName+"\"");
   return factory;
}

public DbConnection GetConnection(String connectionName)
{
   //Get the connection string info from web.config
   ConnectionStringSettings cs = GetConnectionString(connectionName);

   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = GetFactory(cs.ProviderName);


   //Have the factory give us the right connection object
   DbConnection conn = factory.CreateConnection();

   //Undefined behaviour if CreateConnection failed
   //Defensive test for null connection anyway
   if (conn == null)
      throw new Exception("Could not obtain connection from factory");

   //Knowing the connection string, open the connection
   conn.ConnectionString = cs.ConnectionString;
   conn.Open()

   return conn;
}

PD: Esto no es un refactor completo, solo lo hicieron los dos primeros bloques.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top