Parámetro para controlar si lanzar una excepción o devolver nulo: ¿buena práctica?

24

A menudo me encuentro con métodos / funciones que tienen un parámetro booleano adicional que controla si se produce una excepción en caso de error o se devuelve un valor nulo.

Ya hay discusiones sobre cuál es la mejor opción en cada caso, así que no nos centremos en esto aquí. Ver, por ejemplo, ¿ Devolver valor mágico, lanzar excepción o devolver falso en caso de falla?

En su lugar, supongamos que hay una buena razón por la que queremos apoyar en ambos sentidos.

Personalmente, creo que dicho método debería dividirse en dos: uno que arroja una excepción en caso de falla, el otro que devuelve nulo en caso de falla.

Entonces, ¿cuál es mejor?

A: Un método con $exception_on_failureparámetro.

/**
 * @param int $id
 * @param bool $exception_on_failure
 *
 * @return Item|null
 *   The item, or null if not found and $exception_on_failure is false.
 * @throws NoSuchItemException
 *   Thrown if item not found, and $exception_on_failure is true.
 */
function loadItem(int $id, bool $exception_on_failure): ?Item;

B: dos métodos distintos.

/**
 * @param int $id
 *
 * @return Item|null
 *   The item, or null if not found.
 */
function loadItemOrNull(int $id): ?Item;

/**
 * @param int $id
 *
 * @return Item
 *   The item, if found (exception otherwise).
 *
 * @throws NoSuchItemException
 *   Thrown if item not found.
 */
function loadItem(int $id): Item;

EDITAR: C: ¿Algo más?

Muchas personas han sugerido otras opciones o afirman que tanto A como B tienen fallas. Tales sugerencias u opiniones son bienvenidas, relevantes y útiles. Una respuesta completa puede contener dicha información adicional, pero también abordará la pregunta principal de si un parámetro para cambiar la firma / comportamiento es una buena idea.

Notas

En caso de que alguien se pregunte: los ejemplos están en PHP. Pero creo que la pregunta se aplica a todos los lenguajes siempre que sean algo similares a PHP o Java.

Don Quijote
fuente
55
Si bien tengo que trabajar en PHP y soy bastante competente, es simplemente un mal lenguaje, y su biblioteca estándar está por todas partes. Lea eev.ee/blog/2012/04/09/php-a-fractal-of-bad-design para referencia. Así que no es realmente sorprendente que algunos desarrolladores de PHP adopten malos hábitos. Pero todavía no he visto este olor a diseño en particular.
Polygnome
Me falta un punto esencial en las respuestas dadas: cuándo usaría una u otra. Utilizaría el método de excepción en caso de que un elemento que no se encuentre sea inesperado y se considere un error (tiempo de pánico). Usaría el método Probar ... en caso de que un elemento faltante no sea poco probable y ciertamente no sea un error, sino solo una posibilidad que incluya en su control de flujo normal.
Martin Maat
Relacionado: stackoverflow.com/questions/34439782/…
Martin Maat
1
@Polygnome Tiene razón, las funciones de biblioteca estándar están por todas partes, y existe una gran cantidad de código deficiente, y existe el problema de un proceso por solicitud. Pero está mejorando con cada versión. Los tipos y el modelo de objetos hacen todas o la mayoría de las cosas que uno puede esperar de él. Quiero ver genéricos, covarianza y contravarianza, tipos de devolución de llamada que especifiquen una firma, etc. Mucho de esto se está discutiendo o en vías de implementación. Creo que la dirección general es buena, aunque las peculiaridades de la biblioteca estándar no desaparecerán fácilmente.
donquixote
44
Una sugerencia: en lugar de loadItemOrNull(id)considerar si loadItemOr(id, defaultItem)tiene sentido para usted. Si el artículo es una Cadena o número, a menudo lo hace.
user949300

Respuestas:

35

Tienes razón: dos métodos son mucho mejores para eso, por varias razones:

  1. En Java, la firma del método que potencialmente arroja una excepción incluirá esta excepción; el otro método no lo hará. Esto deja particularmente claro qué esperar de uno y otro.

  2. En lenguajes como C # donde la firma del método no dice nada acerca de las excepciones, los métodos públicos aún deben documentarse, y dicha documentación incluye las excepciones. Documentar un solo método no sería fácil.

    Su ejemplo es perfecto: los comentarios en la segunda parte del código se ven mucho más claros, e incluso resumiría "El artículo, si se encuentra (excepción de lo contrario)" hasta "El artículo": la presencia de una posible excepción y el La descripción que le diste se explica por sí misma.

  3. En el caso de un único método, hay pocos casos en los que le gustaría alternar el valor del parámetro booleano en tiempo de ejecución , y si lo hace, significaría que la persona que llama tendrá que manejar ambos casos (una nullrespuesta y la excepción ), lo que hace que el código sea mucho más difícil de lo necesario. Dado que la elección se realiza no en tiempo de ejecución, sino al escribir el código, dos métodos tienen mucho sentido.

  4. Algunos marcos, como .NET Framework, han establecido convenciones para esta situación y la resuelven con dos métodos, tal como usted sugirió. La única diferencia es que usan un patrón explicado por Ewan en su respuesta , por lo que int.Parse(string): intarroja una excepción, mientras int.TryParse(string, out int): boolque no lo hace. Esta convención de nomenclatura es muy sólida en la comunidad de .NET y debe seguirse siempre que el código coincida con la situación que usted describe.

Arseni Mourzenko
fuente
1
¡Gracias, esta es la respuesta que estaba buscando! El propósito fue que comencé una discusión sobre esto para Drupal CMS, drupal.org/project/drupal/issues/2932772 , y no quiero que se trate de mi preferencia personal, pero lo respaldo con los comentarios de otros.
donquixote
44
En realidad, la tradición de larga data de al menos .NET es NO usar dos métodos, sino elegir la opción correcta para la tarea correcta. Las excepciones son para circunstancias excepcionales, no para manejar el flujo de control esperado. ¿No puedes analizar una cadena en un entero? No es una circunstancia muy inesperada o excepcional. int.Parse()se considera una decisión de diseño realmente mala, razón por la cual el equipo de .NET siguió adelante e implementó TryParse.
Voo
1
Proporcionar dos métodos en lugar de pensar en el tipo de caso de uso con el que estamos lidiando, es la salida fácil: no piense en lo que está haciendo, sino que asuma la responsabilidad de todos los usuarios de su API. Claro que funciona, pero ese no es el sello distintivo de un buen diseño de API (o cualquier diseño para el caso. Lea, por ejemplo, Joel sobre esto .
Voo
@Voo Punto válido. De hecho, proporcionar dos métodos pone la opción en la persona que llama. Quizás para algunos valores de parámetros se espera que exista un elemento, mientras que para otros valores de parámetro, un elemento no existente se considera un caso normal. Quizás el componente se use en una aplicación donde siempre se espera que existan los elementos, y en otra aplicación donde los elementos no existentes se consideran normales. Quizás la persona que llama llama ->itemExists($id)antes de llamar ->loadItem($id). O tal vez es solo una elección hecha por otras personas en el pasado, y tenemos que darla por sentado.
donquixote
1
Motivo 1b: algunos idiomas (Haskell, Swift) requieren nulabilidad para estar presentes en la firma. Específicamente, Haskell tiene un tipo completamente diferente para valores anulables ( data Maybe a = Just a | Nothing).
John Dvorak
9

Una variación interesante es el lenguaje Swift. En Swift, declara una función como "lanzamiento", es decir, está permitido lanzar errores (similar pero no exactamente igual a las excepciones de Java). La persona que llama puede llamar a esta función de cuatro maneras:

a. En una declaración try / catch, captura y manejo de la excepción.

si. Si la persona que llama se declara como lanzando, simplemente llámelo, y cualquier error arrojado se convierte en un error lanzado por la persona que llama.

do. Marcar la llamada de función con lo try!que significa "Estoy seguro de que esta llamada no se lanzará". Si la función llamada hace un tiro, la aplicación está garantizada a accidente.

re. Marcar la llamada a la función con lo try?que significa "Sé que la función puede lanzar pero no me importa qué error se lanza exactamente". Esto cambia el valor de retorno de la función de tipo " T" a tipo " optional<T>", y una excepción lanzada se convierte en retorno nulo.

(a) y (d) son los casos sobre los que está preguntando. ¿Qué pasa si la función puede devolver nulo y puede lanzar excepciones? En ese caso, el tipo del valor de retorno sería " optional<R>" para algún tipo R, y (d) cambiaría esto a " optional<optional<R>>", que es un poco críptico y difícil de entender pero completamente correcto. Y una función Swift puede regresar optional<Void>, por lo que (d) puede usarse para lanzar funciones que devuelven Void.

gnasher729
fuente
2

Me gusta el bool TryMethodName(out returnValue)acercamiento.

Le da una indicación concluyente de si el método tuvo éxito o no y los nombres coinciden envolviendo el método de lanzamiento de excepción en un bloque try catch.

Si solo devuelve nulo, no sabe si falló o si el valor de retorno fue legítimamente nulo.

p.ej:

//throws exceptions when error occurs
Item LoadItem(int id);

//returns false when error occurs
bool TryLoadItem(int id, out Item item)

ejemplo de uso (out significa pasar por referencia sin inicializar)

Item i;
if(TryLoadItem(3, i))
{
    Print(i.Name);
}
else
{
    Print("unable to load item :(");
}
Ewan
fuente
Lo siento, me perdiste aquí. ¿Cómo se vería su " bool TryMethodName(out returnValue)enfoque" en el ejemplo original?
donquixote
editado para agregar un ejemplo
Ewan
1
Entonces, ¿utiliza un parámetro de referencia en lugar de un valor de retorno, para tener el valor de retorno libre para el éxito de bool? ¿Existe un lenguaje de programación que admita esta palabra clave "out"?
donquixote
2
sí, lo siento, no me di cuenta de que 'out' es específico de c #
Ewan
1
No siempre no. Pero, ¿y si el elemento 3 es nulo? no sabría si hubo un error o no
Ewan
2

Por lo general, un parámetro booleano indica que una función se puede dividir en dos y, por regla general, siempre debe dividirse en lugar de pasar un booleano.

Max
fuente
1
Por regla general, no lo haría sin agregar alguna calificación o matiz. Eliminar un booleano como argumento puede obligarlo a escribir un tonto if / else en otro lugar y, por lo tanto, codificar datos en su código escrito y no en variables.
Gerard
@Gerard, ¿puedes darme un ejemplo, por favor?
Max
@Max Cuando tiene una variable booleana b: en lugar de llamar foo(…, b);, tiene que escribir if (b) then foo_x(…) else foo_y(…);y repetir los otros argumentos, o algo así como ((b) ? foo_x : foo_y)(…)si el lenguaje tiene un operador ternario y funciones / métodos de primera clase. Y esto puede incluso repetirse desde varios lugares diferentes en el programa.
BlackJack
@BlackJack bueno, sí, podría estar de acuerdo contigo. Pensé en el ejemplo, if (myGrandmaMadeCookies) eatThem() else makeFood()que sí podría envolver en otra función como esta checkIfShouldCook(myGrandmaMadeCookies). Me pregunto si el matiz que mencionó tiene que ver con si estamos pasando un booleano sobre cómo queremos que se comporte la función, como en la pregunta original, frente a si estamos pasando alguna información sobre nuestro modelo, como en el abuela ejemplo que acabo de dar.
Max
1
El matiz mencionado en mi comentario se refiere a "siempre deberías". Tal vez lo reformule como "si a, byc son aplicables, entonces [...]". La "regla" que mencionas es un gran paradigma, pero muy variante en el caso de uso.
Gerard
1

Clean Code recomienda evitar argumentos de bandera booleana, porque su significado es opaco en el sitio de la llamada:

loadItem(id, true) // does this throw or not? We need to check the docs ...

mientras que métodos separados pueden usar nombres expresivos:

loadItemOrNull(id); // meaning is obvious; no need to refer to docs
meriton - en huelga
fuente
1

Si tuviera que usar A o B, usaría B, dos métodos separados, por las razones indicadas en la respuesta de Arseni Mourzenkos .

Pero hay otro método 1 : en Java se llama Optional, pero puede aplicar el mismo concepto a otros lenguajes donde puede definir sus propios tipos, si el lenguaje aún no proporciona una clase similar.

Usted define su método de esta manera:

Optional<Item> loadItem(int id);

En su método, usted regresa Optional.of(item)si encontró el artículo o regresa Optional.empty()en caso de que no lo haga. Nunca lanzas 2 y nunca regresas null.

Para el cliente de su método es algo así null, pero con la gran diferencia que obliga a pensar en el caso de elementos faltantes. El usuario no puede simplemente ignorar el hecho de que puede no haber resultado. Para sacar el elemento del Optionalse requiere una acción explícita:

  • loadItem(1).get()arrojará NoSuchElementExceptiono devolverá el artículo encontrado.
  • También hay loadItem(1).orElseThrow(() -> new MyException("No item 1"))que usar una excepción personalizada si la devolución Optionalestá vacía.
  • loadItem(1).orElse(defaultItem)devuelve el elemento encontrado o el pasado defaultItem(que también puede ser null) sin lanzar una excepción.
  • loadItem(1).map(Item::getName)volvería a devolver un Optionalcon el nombre de los elementos, si está presente.
  • Hay algunos métodos más, consulte el Javadoc paraOptional .

La ventaja es que ahora depende del código del cliente lo que sucederá si no hay un artículo y aún así solo necesita proporcionar un método único .


1 Supongo que es algo así como la respuestatry? en gnasher729s, pero no está del todo claro para mí, ya que no conozco Swift y parece ser una característica del lenguaje, por lo que es específico de Swift.

2 Puede lanzar excepciones por otros motivos, por ejemplo, si no sabe si hay un elemento o no porque tuvo problemas para comunicarse con la base de datos.

siegi
fuente
0

Como otro punto de acuerdo con el uso de dos métodos, esto puede ser muy fácil de implementar. Escribiré mi ejemplo en C #, ya que no conozco la sintaxis de PHP.

public Widget GetWidgetOrNull(int id) {
    // All the lines to get your item, returning null if not found
}

public Widget GetWidget(int id) {
    var widget = GetWidgetOrNull(id);

    if (widget == null) {
        throw new WidgetNotFoundException();
    }

    return widget;
}
krillgar
fuente
0

Prefiero dos métodos, de esta manera, no solo me dirá que está devolviendo un valor, sino qué valor exactamente.

public int GetValue(); // If you can't get the value, you'll throw me an exception
public int GetValueOrDefault(); // If you can't get the value, you'll return me 0, since it's the default for ints

TryDoSomething () tampoco es un mal patrón.

public bool TryParse(string value, out int parsedValue); // If you return me false, I won't bother checking parsedValue.

Estoy más acostumbrado a ver el primero en las interacciones de la base de datos, mientras que el segundo se usa más en el análisis.

Como otros ya te han dicho, los parámetros de la bandera son generalmente un olor a código (los olores a código no significan que estás haciendo algo mal, solo que hay una probabilidad de que lo estés haciendo mal). Aunque lo nombras con precisión, a veces hay matices que dificultan saber cómo usar esa bandera y terminas mirando dentro del cuerpo del método.

Un bravo dev
fuente
-1

Mientras que otras personas sugieren lo contrario, sugeriría que tener un método con un parámetro que indique cómo deben manejarse los errores es mejor que requerir el uso de métodos separados para los dos escenarios de uso. Si bien puede ser deseable tener también puntos de entrada distintos para los usos de "error arroja" versus "error devuelve indicación de error", tener un punto de entrada que pueda servir a ambos patrones de uso evitará la duplicación de código en los métodos de envoltura. Como un simple ejemplo, si uno tiene funciones:

Thing getThing(string x);
Thing tryGetThing (string x);

y uno quiere funciones que se comporten de la misma manera pero con x entre paréntesis, sería necesario escribir dos conjuntos de funciones de contenedor:

Thing getThingWithBrackets(string x)
{
  getThing("["+x+"]");
}
Thing tryGetThingWithBrackets (string x);
{
  return tryGetThing("["+x+"]");
}

Si hay un argumento sobre cómo manejar los errores, solo se necesitaría un contenedor:

Thing getThingWithBrackets(string x, bool returnNullIfFailure)
{
  return getThing("["+x+"]", returnNullIfFailure);
}

Si el contenedor es lo suficientemente simple, la duplicación del código podría no ser tan mala, pero si alguna vez necesita cambiar, duplicar el código a su vez requerirá duplicar cualquier cambio. Incluso si uno opta por agregar puntos de entrada que no usan el argumento adicional:

Thing getThingWithBrackets(string x)
{
   return getThingWithBrackets(x, false);
}
Thing tryGetThingWithBrackets(string x)
{
   return tryGetThingWithBrackets(x, true);
}

se puede mantener toda la "lógica de negocios" en un método: el que acepta un argumento que indica qué hacer en caso de falla.

Super gato
fuente
Tiene razón: los decoradores y envoltorios, e incluso la implementación regular, tendrán cierta duplicación de código cuando usen dos métodos.
donquixote
¿Seguramente colocar las versiones con y sin excepción lanzando en paquetes separados, y hacer que los envoltorios sean genéricos?
Will Crawford
-1

Creo que todas las respuestas que explican que no debe tener una bandera que controle si se produce una excepción o no son buenas. Su consejo sería mi consejo para cualquiera que sienta la necesidad de hacer esa pregunta.

Sin embargo, quería señalar que hay una excepción significativa a esta regla. Una vez que esté lo suficientemente avanzado como para comenzar a desarrollar API que utilizarán cientos de otras personas, hay casos en los que es posible que desee proporcionar dicha marca. Cuando estás escribiendo una API, no solo estás escribiendo a los puristas. Estás escribiendo a clientes reales que tienen deseos reales. Parte de su trabajo es hacerlos felices con su API. Eso se convierte en un problema social, en lugar de un problema de programación, y a veces los problemas sociales dictan soluciones que serían menos que ideales como soluciones de programación por sí mismas.

Es posible que tenga dos usuarios distintos de su API, uno de los cuales quiere excepciones y otro que no. Un ejemplo de la vida real donde esto podría ocurrir es con una biblioteca que se usa tanto en desarrollo como en un sistema integrado. En entornos de desarrollo, los usuarios probablemente deseen tener excepciones en todas partes. Las excepciones son muy populares para manejar situaciones inesperadas. Sin embargo, están prohibidos en muchas situaciones incrustadas porque son demasiado difíciles de analizar en tiempo real. Cuando realmente le importa no solo el tiempo promedio que lleva ejecutar su función, sino también el tiempo que le lleva a cualquier diversión individual, la idea de desenrollar la pila en cualquier lugar arbitrario es muy indeseable.

Un ejemplo de la vida real para mí: una biblioteca de matemáticas. Si su biblioteca de matemáticas tiene una Vectorclase que admite obtener un vector unitario en la misma dirección, debe poder dividir por la magnitud. Si la magnitud es 0, tiene una situación de división por cero. En desarrollo quieres atraparlos . Realmente no quieres sorprender dividir entre 0 dando vueltas. Lanzar una excepción es una solución muy popular para esto. Casi nunca sucede (es un comportamiento verdaderamente excepcional), pero cuando sucede, quieres saberlo.

En la plataforma integrada, no desea esas excepciones. Tendría más sentido hacer un chequeo if (this->mag() == 0) return Vector(0, 0, 0);. De hecho, veo esto en código real.

Ahora piense desde la perspectiva de un negocio. Puede intentar enseñar dos formas diferentes de usar una API:

// Development version - exceptions        // Embeded version - return 0s
Vector right = forward.cross(up);          Vector right = forward.cross(up);
Vector localUp = right.cross(forward);     Vector localUp = right.cross(forward);
Vector forwardHat = forward.unit();        Vector forwardHat = forward.tryUnit();
Vector rightHat = right.unit();            Vector rightHat = right.tryUnit();
Vector localUpHat = localUp.unit()         Vector localUpHat = localUp.tryUnit();

Esto satisface las opiniones de la mayoría de las respuestas aquí, pero desde la perspectiva de la compañía esto no es deseable. Los desarrolladores integrados tendrán que aprender un estilo de codificación, y los desarrolladores de desarrollo tendrán que aprender otro. Esto puede ser bastante molesto y obliga a las personas a una sola forma de pensar. Potencialmente peor: ¿cómo se demuestra que la versión incrustada no incluye ningún código de excepción? La versión de lanzamiento se puede mezclar fácilmente, escondiéndose en algún lugar de su código hasta que una costosa Junta de revisión de fallas lo encuentre.

Si, por otro lado, tiene una bandera que activa o desactiva el manejo de excepciones, ambos grupos pueden aprender exactamente el mismo estilo de codificación. De hecho, en muchos casos, incluso puede usar el código de un grupo en los proyectos del otro grupo. En el caso de este código unit(), si realmente le importa lo que sucede en un caso dividido por 0, debería haber escrito la prueba usted mismo. Por lo tanto, si lo mueve a un sistema integrado, donde solo obtiene malos resultados, ese comportamiento es "sensato". Ahora, desde una perspectiva comercial, entreno a mis codificadores una vez, y usan las mismas API y los mismos estilos en todas las partes de mi negocio.

En la gran mayoría de los casos, desea seguir los consejos de otros: use modismos similares tryGetUnit()o similares para funciones que no arrojen excepciones y, en su lugar, devuelva un valor centinela como nulo. Si cree que los usuarios pueden querer usar tanto el código de manejo de excepciones como el de manejo de excepciones uno al lado del otro dentro de un solo programa, quédese con la tryGetUnit()notación. Sin embargo, hay un caso de esquina donde la realidad de los negocios puede mejorar el uso de una bandera. Si te encuentras en ese caso de esquina, no tengas miedo de tirar las "reglas" por el camino. ¡Para eso están las reglas!

Cort Ammon - Restablece a Monica
fuente