¿Verifique los parámetros anotados con @Nonnull para nulo?

19

Hemos comenzado a usar FindBugs y a anotar nuestros parámetros de manera @Nonnulladecuada, y funciona muy bien para señalar errores al principio del ciclo. Hasta ahora hemos seguido verificando estos argumentos para nullusar Guava's checkNotNull, pero preferiría verificar nullsolo en los bordes, lugares donde el valor puede entrar sin haber sido verificado null, por ejemplo, una solicitud SOAP.

// service layer accessible from outside
public Person createPerson(@CheckForNull String name) {
    return new Person(Preconditions.checkNotNull(name));
}

...

// internal constructor accessed only by the service layer
public Person(@Nonnull String name) {
    this.name = Preconditions.checkNotNull(name);  // remove this check?
}

Entiendo que @Nonnullno bloquea los nullvalores en sí.

Sin embargo, dado que FindBugs señalará en cualquier lugar donde se transfiere un valor de un campo sin marcar a uno marcado @Nonnull, ¿no podemos depender de él para detectar estos casos (lo que hace) sin tener que verificar estos valores en nulltodos los lugares en los que se pasan? ¿el sistema? ¿Soy ingenuo para querer confiar en la herramienta y evitar estos controles detallados?

En pocas palabras: Si bien parece seguro eliminar la segunda nullverificación a continuación, ¿es una mala práctica?

Esta pregunta es quizás demasiado similar a ¿Debería uno verificar nulo si no espera nulo , pero lo pregunto específicamente en relación con la @Nonnullanotación?

David Harkness
fuente

Respuestas:

6

Si no confía en FindBug, puede ser una opción usar una aserción. De esta manera, evita los problemas mencionados por el usuario MSalters pero aún tiene un cheque. Por supuesto, este enfoque puede no ser aplicable si dicho enfoque de falla rápida no es factible, es decir, debe detectar alguna excepción.

Y para responder aún más a la pregunta de si es una mala práctica. Bueno, esto es discutible pero no lo creo. Considero esta anotación FindBug como una especificación ligera que sigue el diseño por principio de contrato.

En el diseño por contrato, establece un contrato entre un método y su interlocutor. El contrato consta de condiciones previas que la persona que llama acuerda cumplir al llamar al método y una condición posterior que a su vez se cumple con el método después de la ejecución si se cumple su condición previa.

Uno de los beneficios de este método de desarrollo es que puede evitar el llamado estilo de programación defensiva (en el que verifica explícitamente todos los valores de parámetros no válidos, etc.). En cambio, puede confiar en el contrato y evitar verificaciones redundantes para aumentar el rendimiento y la legibilidad.

Los contratos se pueden usar para la verificación de afirmación de tiempo de ejecución que sigue el principio de falla primero mencionado anteriormente en el que desea que su programa falle si se rompe un contrato, lo que le da una pista para identificar la fuente del error. (Una condición previa fallida significa que la persona que llamó hizo algo mal, una condición posterior fallida indica un error de implementación del método dado)

Además, los contratos se pueden utilizar para el análisis estático, que intenta sacar conclusiones sobre el código fuente sin ejecutarlo realmente.

La anotación @nonnull puede verse como una condición previa que la herramienta FindBugs utiliza para el análisis estático. Si prefiere un enfoque como la verificación de aserción en tiempo de ejecución, es decir, la estrategia de falla primero, debería considerar el uso de declaraciones de aserción como Java incorporado. O tal vez para adaptarse a una estrategia de especificación más sofisticada utilizando un lenguaje de especificación de interfaz de comportamiento como el Java Modeling Language (JML).

JML es una extensión de Java en la que la especificación está incrustada en comentarios especiales de Java. Una ventaja de este enfoque es que puede usar especificaciones sofisticadas, incluso usando un enfoque de desarrollo en el que solo confía en la especificación en lugar de en los detalles de implementación y usa diferentes herramientas que hacen uso de la especificación, como las herramientas mencionadas de verificación de afirmación de tiempo de ejecución, Generación automatizada de pruebas unitarias o documentación.

Si comparte mi punto de vista de que @nonnull es un contrato (ligero), sería una práctica común no agregar controles adicionales porque la idea original detrás de este principio es evitar la programación defensiva.

FabianB
fuente
2
Así es exactamente como estoy usando @Nonnull: como una especificación de contrato. He estado eliminando los controles defensivos detrás del contrato y hasta ahora no he tenido problemas.
David Harkness
4

Bueno, considera por qué no escribes

this.name = Preconditions.checkNotNull(name);  // Might be necessary
that.name = Preconditions.checkNotNull(this.name);  // Who knows?

La programación no es magia negra. Las variables no se vuelven nulas de repente. Si sabe que una variable no es nula y sabe que no ha cambiado, entonces no la marque.

Si lo verifica, algún programador en el futuro (posiblemente usted) pasará mucho tiempo descubriendo por qué ese control está allí. El cheque debe estar allí por una razón, después de todo, entonces, ¿qué estás pasando por alto?

MSalters
fuente
3
El @Nonnullcontrato estipula que las personas que llaman no deben pasar nullal constructor, y FindBugs utiliza un análisis estático para localizar llamadas que podrían permitir, nullespecíficamente llamadas que pasan valores que no están marcados @Nonnull. Pero una persona que llama es libre de pasar nulle ignorar la advertencia de FindBugs.
David Harkness
La programación idealmente no es magia negra. Desafortunadamente, abundan las advertencias y sorpresas, especialmente cuando se trabaja con algunos de los códigos de concurrencia disponibles ♬ ~ ᕕ (ᐛ) ᕗ
Tim Harper