2008-10-13 7 views
6

El título puede que no explique realmente a qué estoy tratando de llegar, realmente no podría pensar en una manera de describir lo que quiero decir.¿Debo asegurarme de que los argumentos no sean nulos antes de usarlos en una función?

Me preguntaba si es una buena práctica verificar los argumentos que una función acepta para nulos o vacíos antes de usarlos. Tengo esta función que simplemente envuelve algo de creación de hash como tal.

Public Shared Function GenerateHash(ByVal FilePath As IO.FileInfo) As String 
     If (FilePath Is Nothing) Then 
      Throw New ArgumentNullException("FilePath") 
     End If 

     Dim _sha As New Security.Cryptography.MD5CryptoServiceProvider 
     Dim _Hash = Convert.ToBase64String(_sha.ComputeHash(New IO.FileStream(FilePath.FullName, IO.FileMode.Open, IO.FileAccess.Read))) 
     Return _Hash 
    End Function 

Como se puede ver que sólo se necesita un IO.Fileinfo como argumento, en el inicio de la función estoy comprobando para asegurarse de que no hay nada.

Me pregunto si esta es una buena práctica o debería dejar que llegue al hasher real y luego lanzar la excepción porque es nula.

Gracias.

Respuesta

20

En general, sugiero que es una buena práctica validar todos los argumentos a las funciones/métodos públicos antes de usarlos, y fallar temprano en lugar de después de ejecutar la mitad de la función. En este caso, tiene razón para lanzar la excepción.

Dependiendo de lo que esté haciendo su método, fallar temprano podría ser importante. Si su método estaba alterando datos de instancia en su clase, no desea que altere la mitad de los datos, luego encuentra el nulo y lanza una excepción, ya que los datos de su objeto podrían estar en un estado intermedio y posiblemente inválido.

Si está utilizando un lenguaje OO, le sugiero que es esencial para validar los argumentos a los métodos públicos, pero menos importante con los métodos privados y protegidos. Mi razonamiento aquí es que usted no sabe cuáles serán las entradas a un método público; cualquier otro código podría crear una instancia de su clase y llamar a sus métodos públicos, y pasar datos inesperados/no válidos. Los métodos privados, sin embargo, se llaman desde dentro de la clase, y la clase ya debería haber validado cualquier dato que pase internamente.

1

Si NULL es una entrada inaceptable, ejecute una excepción. Por ti mismo, como lo hiciste en tu muestra, para que el mensaje sea útil.

Otro método para manejar las entradas NULL es simplemente para responder con un NULL a su vez. Depende del tipo de función: en el ejemplo anterior, mantendría la excepción.

1

Si se trata de una API externa, diría que desea verificar cada parámetro, ya que no se puede confiar en la entrada.

Sin embargo, si solo se va a utilizar internamente, la entrada debe ser confiable y puede ahorrarse un montón de código que no agrega valor al software.

0

La mayoría de las veces, dejar que solo arroje la excepción es bastante razonable siempre que esté seguro de que la excepción no se ignorará.

Si puede agregarle algo, sin embargo, no hace daño envolver la excepción con una que sea más precisa y volver a lanzarla. La decodificación de "NullPointerException" tardará un poco más que "IllegalArgumentException (" FilePath DEBE incluirse ")" (O lo que sea).

Últimamente he estado trabajando en una plataforma donde tienes que ejecutar un ofuscador antes de realizar la prueba. Cada traza de pila parece monos escribiendo basura al azar, así que tuve la costumbre de verificar mis argumentos todo el tiempo.

Me encantaría ver un modificador "anulable" o "no válido" en variables y argumentos para que el compilador pueda verificar por usted.

2

Sí, es una buena práctica validar todos los argumentos al comienzo de un método y arrojar excepciones apropiadas como ArgumentException, ArgumentNullException o ArgumentOutOfRangeException.

Si el método es privado, de modo que solo el programador puede pasar argumentos no válidos, puede optar por afirmar que cada argumento es válido (Debug.Assert) en lugar de throw.

3

Una de mis técnicas favoritas en C++ fue DEBUG_ASSERT en punteros NULL. Esto me lo enseñaron los programadores senior (junto con la precisión) y es una de las cosas en las que fui más estricto durante las revisiones del código. Nosotros nunca desmarcamos un puntero sin primero afirmar que no era nulo.

Una afirmación de depuración solo está activa para los objetivos de depuración (se elimina en la versión) por lo que no tiene la sobrecarga adicional en producción para probar miles de if. Generalmente arrojaría una excepción o activaría un punto de corte de hardware. Incluso teníamos sistemas que lanzaban una consola de depuración con la información de archivo/línea y una opción para ignorar la afirmación (una o indefinidamente para la sesión). Esa fue una gran herramienta de depuración y control de calidad (obtendríamos capturas de pantalla con la afirmación en la pantalla del verificador e información sobre si el programa continuaba si no se hacía caso).

Sugiero que se afirmen todas las invariantes en su código, incluidos los nulos inesperados. Si el rendimiento del if se convierte en una preocupación, encuentre una forma de compilarlos condicionalmente y mantenerlos activos en los objetivos de depuración. Al igual que el control de fuente, esta es una técnica que me ha salvado el trasero más a menudo de lo que me ha causado dolor (la prueba de fuego más importante de cualquier técnica de desarrollo).

0

Si está escribiendo una API pública, haga que su interlocutor reciba el favor de ayudarlo a encontrar sus errores rápidamente, y verifique si tiene entradas válidas.

Si está escribiendo una API donde la persona que llama podría no ser de confianza (o la persona que llama de la persona que llama), verificó las entradas válidas, porque es una buena seguridad.

Si tus llamadas solo son accesibles por personas que llaman de confianza, como "internas" en C#, entonces no sientes que tienes que escribir todo ese código extra. No será útil para nadie.

1

Debe verificar todos los argumentos contra el conjunto de suposiciones que realiza en esa función sobre sus valores.

Como en su ejemplo, si un argumento nulo para su función no tiene ningún sentido y está asumiendo que cualquiera que use su función sabrá esto, entonces un argumento nulo muestra algún tipo de error y algún tipo de acción tomada (ej. lanzando una excepción). Y si usa afirmaciones (como dijo James Fassett y dijo antes que yo ;-)) no le costaron nada en una versión de lanzamiento. (tampoco le cuestan casi nada en una versión de depuración)

Lo mismo se aplica a cualquier otra suposición.

Y va a ser más fácil rastrear el error si lo genera que si deja alguna rutina de biblioteca estándar para lanzar la excepción. Podrá proporcionar información contextual mucho más útil.

Está fuera de los límites de esta pregunta, pero debe exponer las suposiciones que realiza su función, por ejemplo, a través del encabezado de comentario de su función.

1

De acuerdo con El programador pragmático por Andrew Hunt y David Thomas, es la responsabilidad de la persona que llama para asegurarse de que da una entrada válida. Por lo tanto, ahora debe elegir si considera que una entrada nula es válida. A menos que tenga sentido específico considerar nulo como una entrada válida (por ejemplo, es probable que sea una buena idea considerar nulo como una entrada legal si está probando la igualdad), lo consideraría no válido. De esta forma, su programa, cuando ingrese información incorrecta, fallará antes. Si su programa va a encontrar una condición de error, quiere que suceda lo antes posible. En el caso de que su función pase inadvertidamente a un valor nulo, debería considerarlo como un error y reaccionar en consecuencia (es decir, en lugar de lanzar una excepción, debería considerar hacer uso de una afirmación que mata el programa, hasta que libere el programa).

Diseño clásico por contrato: si la entrada es correcta, la salida será correcta. Si la entrada es incorrecta, hay un error. (Si la entrada es correcta pero la salida es incorrecta, hay un error. Es un truco.)

+0

y si la entrada es incorrecta y la salida es correcta ... hrm ... eso tampoco es tan bueno. – stephenbayer

1

Agregaré un par de elaboraciones (en negrita) al excelente diseño por contrato ofrecido por Brian anteriormente. ..

Los principios de "diseño por contrato" requieren que usted defina qué es aceptable para el que llama (el dominio válido de valores de entrada) y luego, para cualquier entrada válida, lo que el método/proveedor hará .

Para un método interno, puede definir valores NULL como fuera del dominio de parámetros de entrada válidos. En este caso, afirmaría inmediatamente que el valor del parámetro de entrada NO ES NULO. La idea clave en esta especificación de contrato es que cualquier llamada que pase en un valor NULO ES UN ERROR DEL LLAMADOR y el error arrojado por la declaración de afirmación es el comportamiento correcto.

Ahora, aunque está muy bien definido y parsimonius, si está exponiendo el método a las llamadas externas/públicas, debería preguntarse, ¿es el contrato lo que realmente queremos? Probablemente no. En una interfaz pública, probablemente acepte el NULL (como técnicamente en el dominio de las entradas que acepta el método), pero luego rechace procesarlo correctamente con un mensaje de devolución. (Más trabajo para cumplir con el requisito de cara al cliente naturalmente más complejo.)

En cualquier caso, lo que está buscando es un protocolo que maneje todos los casos, tanto desde la perspectiva del llamante como del proveedor, no muchas pruebas dispersas que pueden dificultar la evaluación de la integridad o la falta de integridad de la cobertura de la condición contractual.

Cuestiones relacionadas