¿Debo validar el valor de retorno de una llamada al método incluso si sé que el método no puede devolver una entrada incorrecta?

30

Me pregunto si debería defenderme contra el valor de retorno de una llamada al método al validar que cumplan mis expectativas, incluso si sé que el método al que llamo cumplirá tales expectativas.

DADO

User getUser(Int id)
{
    User temp = new User(id);
    temp.setName("John");

    return temp;
}

DEBERIA HACER

void myMethod()
{
    User user = getUser(1234);
    System.out.println(user.getName());
}

O

void myMethod()
{
    User user = getUser(1234);

    // Validating
    Preconditions.checkNotNull(user, "User can not be null.");
    Preconditions.checkNotNull(user.getName(), "User's name can not be null.");

    System.out.println(user.getName());
}

Estoy preguntando esto a nivel conceptual. Si conozco el funcionamiento interno del método que estoy llamando. Ya sea porque lo escribí o lo inspeccioné. Y la lógica de los posibles valores que devuelve cumple mis condiciones previas. ¿Es "mejor" o "más apropiado" omitir la validación, o aún debería defenderme de los valores incorrectos antes de continuar con el método que estoy implementando actualmente, incluso si siempre debe pasar?


Mi conclusión de todas las respuestas (siéntase libre de responder):

Afirmar cuando
  • El método ha demostrado comportarse mal en el pasado
  • El método es de una fuente no confiable
  • El método se usa desde otros lugares y no establece explícitamente sus condiciones posteriores.
No afirmar cuando:
  • El método está muy cerca del suyo (vea la respuesta elegida para más detalles)
  • El método define explícitamente su contrato con algo como el documento adecuado, la seguridad de tipo, una prueba unitaria o una verificación posterior a la condición
  • El rendimiento es crítico (en cuyo caso, una afirmación de modo de depuración podría funcionar como un enfoque híbrido)
Didier A.
fuente
1
¡Apunte a esa cobertura de código% 100!
Jared Burrows
99
¿Por qué te enfocas en un solo problema ( Null)? Probablemente sea igualmente problemático si el nombre es ""(no nulo pero la cadena vacía) o "N/A". O puedes confiar en el resultado, o tienes que ser apropiadamente paranoico.
MSalters
3
En nuestra base de código tenemos un esquema de nombres: getFoo () promete no devolver nulo, y getFooIfPresent () puede devolver nulo.
pjc50
¿De dónde viene su presunción de cordura de código? ¿Asume que el valor siempre será no nulo porque se valida en la entrada o por la estructura por la cual se recupera el valor? Y, lo que es más importante, ¿está probando cada posible escenario de caso límite? ¿Incluyendo la posibilidad de script malicioso?
Zibbobz
@MSalters No valide el valor si está allí ;)
Izkata

Respuestas:

42

Eso depende de la probabilidad de que getUser y myMethod cambien y, lo que es más importante, de la probabilidad de que cambien independientemente uno del otro .

Si de alguna manera sabe con certeza que getUser nunca cambiará en el futuro, entonces sí, es una pérdida de tiempo validarlo, tanto como perder el tiempo de validación que itiene un valor de 3 inmediatamente después de una i = 3;declaración. En realidad, no lo sabes. Pero hay algunas cosas que sí sabes:

  • ¿Estas dos funciones "viven juntas"? En otras palabras, ¿tienen las mismas personas que los mantienen, son parte del mismo archivo y, por lo tanto, es probable que se mantengan "sincronizados" por sí mismos? En este caso, probablemente sea excesivo agregar verificaciones de validación, ya que eso simplemente significa más código que tiene que cambiar (y potencialmente generar errores) cada vez que las dos funciones cambian o se refactorizan en un número diferente de funciones.

  • ¿La función getUser es parte de una API documentada con un contrato específico, y myMethod es simplemente un cliente de dicha API en otra base de código? Si es así, puede leer esa documentación para averiguar si debe validar los valores de retorno (o validar previamente los parámetros de entrada) o si realmente es seguro seguir ciegamente el camino feliz. Si la documentación no lo aclara, pídale al encargado que lo arregle.

  • Finalmente, si esta función en particular ha cambiado repentina e inesperadamente su comportamiento en el pasado, de una manera que rompió su código, tiene todo el derecho de ser paranoico al respecto. Los errores tienden a agruparse.

Tenga en cuenta que todo lo anterior se aplica incluso si usted es el autor original de ambas funciones. No sabemos si se espera que estas dos funciones "vivan juntas" por el resto de sus vidas, o si se separarán lentamente en módulos separados, o si de alguna manera te has disparado en el pie con un error en una versión anterior versiones de getUser. Pero probablemente puedas hacer una suposición bastante decente.

Ixrec
fuente
2
Además: si getUsertuviera que cambiar más tarde, para que pudiera volver nulo, ¿la verificación explícita le daría información que no podría obtener de "NullPointerException" y el número de línea?
user253751
Parece que hay dos cosas que puede validar: (1) Que el método funciona correctamente, ya que no tiene errores. (2) Que el método respeta su contrato. Puedo ver que la validación del n. ° 1 es exagerada. Deberías hacerte pruebas haciendo el # 1 en su lugar. Es por eso que no validaría i = 3 ;. Entonces mi pregunta se relaciona con el n. ° 2. Me gusta tu respuesta para eso, pero al mismo tiempo, es una respuesta útil. ¿Ve algún otro problema que surja de validar explícitamente sus contratos además de perder un poco de tiempo escribiendo la afirmación?
Didier A.
El único otro "problema" es que si su interpretación del contrato es incorrecta o se vuelve incorrecta, debe cambiar toda la validación. Pero eso también se aplica a todos los demás códigos.
Ixrec
1
No puedo ir contra la multitud. Esta respuesta es definitivamente la más correcta para cubrir el tema de mi pregunta. Me gustaría agregar que al leer todas las respuestas, especialmente @MikeNakis, ahora creo que debería hacer valer sus condiciones previas, al menos en modo de depuración, cuando tenga cualquiera de los dos últimos puntos de Ixrec. Si se enfrenta al primer punto, probablemente sea excesivo afirmar sus condiciones previas. Finalmente, dado un contrato explícito, como el documento adecuado, la seguridad de tipo, una prueba de unidad o una verificación posterior a la condición, no debe hacer valer sus condiciones previas, eso sería una exageración.
Didier A.
22

La respuesta de Ixrec es buena, pero adoptaré un enfoque diferente porque creo que vale la pena considerarlo. A los fines de esta discusión, hablaré sobre afirmaciones, ya que ese es el nombre con el que Preconditions.checkNotNull()tradicionalmente se conoce.

Lo que me gustaría sugerir es que los programadores a menudo sobreestiman el grado en que están seguros de que algo se comportará de cierta manera y subestiman las consecuencias de que no se comporte de esa manera. Además, los programadores a menudo no se dan cuenta del poder de las afirmaciones como documentación. Posteriormente, en mi experiencia, los programadores afirman las cosas con mucha menos frecuencia de lo que deberían.

Mi lema es:

La pregunta que siempre debe hacerse no es "¿debería afirmar esto?" pero "¿hay algo que olvidé afirmar?"

Naturalmente, si está absolutamente seguro de que algo se comporta de cierta manera, se abstendrá de afirmar que de hecho se comportó de esa manera, y eso es en su mayor parte razonable. Realmente no es posible escribir software si no puedes confiar en nada.

Pero hay excepciones incluso a esta regla. Curiosamente, para tomar el ejemplo de Ixrec, existe un tipo de situación en la que es perfectamente deseable seguir int i = 3;con una afirmación que iesté dentro de cierto rango. Esta es la situación en la que ies probable que sea alterada por alguien que está probando varios escenarios hipotéticos, y el código que sigue depende de itener un valor dentro de un cierto rango, y no es inmediatamente obvio mirando rápidamente el siguiente código. El rango aceptable es. Entonces, int i = 3; assert i >= 0 && i < 5;al principio puede parecer absurdo, pero si lo piensas, te dice que puedes jugar iy también te dice el rango dentro del cual debes quedarte. Una afirmación es el mejor tipo de documentación, porquela máquina lo aplica, por lo que es una garantía perfecta y se refactoriza junto con el código , por lo que sigue siendo pertinente.

Su getUser(int id)ejemplo no es un pariente conceptual muy distante del assign-constant-and-assert-in-rangeejemplo. Verá, por definición, los errores ocurren cuando las cosas exhiben un comportamiento que es diferente del comportamiento que esperábamos. Es cierto que no podemos afirmar todo, por lo que a veces habrá alguna depuración. Pero es un hecho bien establecido en la industria que cuanto más rápido detectamos un error, menos cuesta. Las preguntas que debe hacer no son solo qué tan seguro está de que getUser()nunca devolverá nulo (y nunca devolverá a un usuario con un nombre nulo), sino también, qué tipo de cosas malas sucederán con el resto del código si lo hace en De hecho, un día devuelve algo inesperado, y lo fácil que es mirar rápidamente el resto del código para saber con precisión lo que se esperaba getUser().

Si el comportamiento inesperado hace que su base de datos se corrompa, entonces tal vez debería afirmar incluso si está 101% seguro de que no sucederá. Si un nullnombre de usuario causara un extraño error críptico imposible de rastrear miles de líneas más abajo y miles de millones de ciclos de reloj más tarde, entonces quizás sea mejor fallar lo antes posible.

Por lo tanto, aunque no estoy en desacuerdo con la respuesta de Ixrec, te sugiero que consideres seriamente el hecho de que las afirmaciones no cuestan nada, por lo que realmente no hay nada que perder, solo que ganar, al usarlas libremente.

Mike Nakis
fuente
2
Sí. Afirmalo. Vine aquí para dar más o menos esta respuesta. ++
RubberDuck
2
@SteveJessop Yo corregiría esto ... && sureness < 1).
Mike Nakis
2
@MikeNakis: eso siempre es una pesadilla al escribir pruebas unitarias, el valor de retorno permitido por la interfaz pero imposible de generar a partir de cualquier entrada ;-) De todos modos, cuando estás 101% seguro de que estás en lo cierto, definitivamente estás equivocado !
Steve Jessop
2
+1 por señalar cosas que olvidé por completo mencionar en mi respuesta.
Ixrec
1
@DavidZ Eso es correcto, mi pregunta asumió la validación del tiempo de ejecución. Pero decir que no confía en el método de depuración y confía en él en prod es posiblemente un buen punto de vista sobre el tema. Entonces, una afirmación de estilo C podría ser una buena manera de lidiar con esta situación.
Didier A.
8

Un definitivo no.

Una persona que llama nunca debe verificar si la función a la que llama respeta su contrato. La razón es simple: potencialmente hay muchas personas que llaman y es poco práctico y posiblemente incluso erróneo desde un punto de vista conceptual para cada persona que llama duplicar la lógica para verificar los resultados de otras funciones.

Si se van a realizar verificaciones, cada función debe verificar sus propias condiciones posteriores. Las condiciones posteriores le dan la confianza de que implementó la función correctamente y los usuarios podrán confiar en el contrato de su función.

Si una determinada función no está destinada a devolver un valor nulo, esto debe documentarse y formar parte del contrato de esa función. Este es el objetivo de estos contratos: ofrecer a los usuarios algunos invariantes en los que puedan confiar para que enfrenten menos obstáculos al escribir sus propias funciones.

Paul
fuente
Estoy de acuerdo con el sentimiento general, pero hay circunstancias en las que definitivamente tiene sentido validar los valores de retorno. Por ejemplo, si llama a un servicio web externo, a veces desea al menos validar el formato de la respuesta (xsd, etc.)
AK_
Parece que estás abogando por un estilo de diseño por contrato sobre uno de programación defensiva. Creo que esta es una gran respuesta. Si el método tuviera un contrato estricto, podría confiar en él. En mi caso no es así, pero podría cambiarlo para hacerlo. ¿Pero no puedo decir? ¿Argumentaría que debería confiar en la implementación? ¿Incluso si no declara explícitamente en su documento que no devuelve nulo o que en realidad tiene una verificación posterior a la condición?
Didier A.
En mi opinión, debes usar tu mejor criterio y ponderar diferentes aspectos como cuánto confías en que el autor haga lo correcto, qué tan probable es que cambie la interfaz de la función y cuán ajustadas son tus limitaciones de tiempo. Dicho esto, la mayoría de las veces, el autor de una función tiene una idea bastante clara de cuál debería ser el contrato de la función, incluso si no la documenta. Debido a esto, digo que primero debe confiar en los autores para que hagan lo correcto y solo ponerse a la defensiva cuando sepa que la función no está bien escrita.
Paul
Creo que confiar primero en los demás para hacer lo correcto me ayuda a hacer el trabajo más rápido. Dicho esto, el tipo de aplicaciones que escribo son fáciles de solucionar cuando se produce un problema. Alguien que trabaje con un software más crítico para la seguridad podría pensar que soy demasiado indulgente y preferiría un enfoque más defensivo.
Paul,
5

Si nulo es un valor de retorno válido del método getUser, entonces su código debe manejar eso con un If, no una excepción; no es un caso excepcional.

Si nulo no es un valor de retorno válido del método getUser, entonces hay un error en getUser: el método getUser debería arrojar una excepción o, de lo contrario, corregirse.

Si el método getUser es parte de una biblioteca externa o no se puede cambiar y desea que se generen excepciones en el caso de un retorno nulo, ajuste la clase y el método para realizar las comprobaciones y arroje la excepción para que cada instancia de la llamada a getUser es consistente en su código.

MatthewC
fuente
Nulo no es un valor de retorno válido de getUser. Al inspeccionar getUser, vi que la implementación nunca volvería nula. Mi pregunta es, ¿debería confiar en mi inspección y no tener un cheque en mi método, o debería dudar de mi inspección y todavía tener un cheque? También es posible que el método cambie en el futuro, rompiendo mis expectativas de no volver nulo.
Didier A.
¿Y si getUser o user.getName cambian para generar excepciones? Debería tener un cheque, pero no como parte del método que utiliza el Usuario. Envuelva el método getUser, e incluso la clase User, para hacer cumplir sus contratos, si le preocupan los cambios futuros en la ruptura del código getUser. También cree pruebas unitarias para verificar sus suposiciones sobre el comportamiento de la clase.
MatthewC
@didibus Parafraseando su respuesta ... Un emoticón sonriente no es un valor de retorno válido de getUser. Al inspeccionar getUser, vi que la implementación nunca devolvería un emoticón sonriente . Mi pregunta es, ¿debería confiar en mi inspección y no tener un cheque en mi método, o debería dudar de mi inspección y todavía tener un cheque? También es posible que el método cambie en el futuro, rompiendo mis expectativas de no devolver un emoticón sonriente . No es tarea del llamante confirmar que la función llamada está cumpliendo su contrato.
Vince O'Sullivan
@ VinceO'Sullivan Parece que el "contrato" está en el centro del problema. Y que mi pregunta es sobre contratos implícitos, versus explícitos. Un método que devuelve int no afirmaré que no estoy obteniendo una cadena. Pero, si solo quiero entradas positivas, ¿debería? Solo está implícito que el método no devuelve int negativo, porque lo inspeccioné. No está claro por la firma o la documentación que no lo hace.
Didier A.
1
No importa si el contrato es implícito o explícito. El trabajo del código de llamada no es validar que el método llamado devuelve datos válidos cuando se le dan datos válidos. En su ejemplo, sabe que los inits negativos no son respuestas incorrectas, pero ¿sabe si los int positivos son correctos? ¿Cuántas pruebas necesitas realmente para demostrar que el método funciona? El único curso correcto es asumir que el método es correcto.
Vince O'Sullivan
5

Yo diría que la premisa de su pregunta es incorrecta: ¿por qué usar una referencia nula para comenzar?

El patrón de objeto nulo es una forma de devolver objetos que esencialmente no hacen nada: en lugar de devolver nulo para su usuario, devuelva un objeto que representa "ningún usuario".

Este usuario estaría inactivo, tendría un nombre en blanco y otros valores no nulos que indicarían "nada aquí". El beneficio principal es que no necesita tirar basura, su programa anulará cheques, registros, etc., simplemente use sus objetos.

¿Por qué un algoritmo debería preocuparse por un valor nulo? Los pasos deben ser los mismos. Es igual de fácil y mucho más claro tener un objeto nulo que devuelva cero, cadena en blanco, etc.

Aquí hay un ejemplo de comprobación de código para nulo:

User u = getUser();
String displayName = "";
if (u != null) {
  displayName = u.getName();
}
return displayName;

Aquí hay un ejemplo de código que usa un objeto nulo:

return getUser().getName();

¿Cuál es más claro? Creo que el segundo es.


Si bien la pregunta está etiquetada como , vale la pena señalar que el patrón de objeto nulo es realmente útil en C ++ cuando se usan referencias. Las referencias de Java son más parecidas a los punteros de C ++ y permiten valores nulos. Las referencias de C ++ no pueden sostenerse nullptr. Si a uno le gustaría tener algo análogo a una referencia nula en C ++, el patrón de objeto nulo es la única forma que conozco para lograrlo.

Comunidad
fuente
Nulo es solo el ejemplo de lo que afirmo. Bien podría ser que necesito un nombre no vacío. Mi pregunta aún se aplicaría, ¿lo validarías? O podría ser algo que devuelve un int, y no puedo tener un int negativo, o debe estar en un rango dado. No piense en ello como un problema nulo por decir.
Didier A.
1
¿Por qué un método devolvería un valor incorrecto? El único lugar donde debe verificarse es en el límite de su aplicación: interfaces con el usuario y con otros sistemas. De lo contrario, es por eso que tenemos excepciones.
Imagina que tengo: int i = getCount; flotador x = 100 / i; Si sé que getCount no devuelve 0, porque escribí el método o lo inspeccioné, ¿debo omitir la comprobación de 0?
Didier A.
@didibus: puede omitir el cheque si getCount() documenta una garantía de que ahora no devuelve 0, y no lo hará en el futuro, excepto en circunstancias en las que alguien decida hacer un cambio importante y busque actualizar todo lo que llama para hacer frente a La nueva interfaz. Ver lo que hace el código actualmente no es de ayuda, pero si va a inspeccionar el código, entonces el código a inspeccionar es la unidad de prueba getCount(). Si prueban que no devuelve 0, entonces sabe que cualquier cambio futuro para devolver 0 se considerará un cambio importante porque las pruebas fallarán.
Steve Jessop
No me queda claro que los objetos nulos son mejores que los nulos. Un pato nulo puede graznar, pero no es un pato; de hecho, semánticamente, es la antítesis de un pato. Una vez que introduce un objeto nulo, cualquier código que use esa clase puede tener que verificar si alguno de sus objetos es el objeto nulo; por ejemplo, si tiene algo nulo en un conjunto, ¿cuántas cosas tiene? Esto me parece una forma de aplazar el fracaso y ofuscar los errores. El artículo vinculado advierte específicamente contra el uso de objetos nulos "solo para evitar verificaciones nulas y hacer que el código sea más legible".
sdenham
1

En general, diría que depende principalmente de los siguientes tres aspectos:

  1. robustez: ¿puede el método de llamada hacer frente al nullvalor? Si un nullvalor pudiera resultar en un RuntimeException, recomendaría una verificación, a menos que la complejidad sea baja y los métodos llamados / invocadores sean creados por el mismo autor y nullno se esperen (por ejemplo, ambos privateen el mismo paquete).

  2. responsabilidad del código: si el desarrollador A es responsable del getUser()método y el desarrollador B lo usa (por ejemplo, como parte de una biblioteca), recomiendo encarecidamente validar su valor. Solo porque el desarrollador B podría no saber acerca de un cambio que resulte en un posible nullvalor de retorno.

  3. complejidad: cuanto mayor es la complejidad general del programa o entorno, más recomiendo validar el valor de retorno. Incluso si está seguro de que no puede ser nullhoy en el contexto del método de llamada, puede ser que tenga que cambiar getUser()por otro caso de uso. Una vez que pasen algunos meses o años y se hayan agregado miles de líneas de códigos, esto puede ser una trampa.

Además, recomendaría documentar los posibles nullvalores de retorno en un comentario JavaDoc. Debido al resaltado de las descripciones de JavaDoc en la mayoría de los IDE, esto puede ser una advertencia útil para quien lo use getUser().

/** Returns ....
  * @param: id id of the user
  * @return: a User instance related to the id;
  *          null, if no user with identifier id exists
 **/
User getUser(Int id)
{
    ...
}
André
fuente
-1

Definitivamente no tienes que hacerlo.

Por ejemplo, si ya sabe que un retorno nunca puede ser nulo, ¿por qué querría agregar un cheque nulo? Agregar un cheque nulo no va a romper nada, pero es redundante. Y mejor no codifique la lógica redundante siempre que pueda evitarla.

Yellen
fuente
1
Lo sé, pero solo porque inspeccioné o escribí la implementación del otro método. El contrato del método no dice explícitamente que no puede. Por lo tanto, podría, por ejemplo, cambiar en el futuro. O podría tener un error que hace que un valor nulo regrese incluso si intenta no hacerlo.
Didier A.