Evaluación de cortocircuito, ¿es una mala práctica?

Algo que he sabido por un tiempo pero que nunca he considerado es que en la mayoría de los idiomas es posible dar prioridad a los operadores en una declaración if basada en su orden. A menudo uso esto como una forma de evitar excepciones de referencia nula, por ejemplo:

if (smartphone != null && smartphone.GetSignal() > 50)
{
   // Do stuff
}

En este caso, el código resultará primero en verificar que el objeto no sea nulo y luego usar este objeto sabiendo que existe. El lenguaje es inteligente porque sabe que si la primera instrucción es falsa, entonces no tiene sentido evaluar la segunda instrucción y, por lo tanto, nunca se produce una excepción de referencia nula. Esto funciona igual para andy los oroperadores.

Esto también puede ser útil en otras situaciones, como verificar que un índice esté dentro de los límites de una matriz y que dicha técnica se pueda realizar en varios idiomas, que yo sepa: Java, C #, C ++, Python y Matlab.

Mi pregunta es: ¿Es este tipo de código una representación de una mala práctica? ¿Esta mala práctica surge de algún problema técnico oculto (es decir, esto podría dar lugar a un error) o conduce a un problema de legibilidad para otros programadores? ¿Podría ser confuso?

No, esto no es una mala práctica. Confiar en el cortocircuito de los condicionales es una técnica útil y ampliamente aceptada, siempre que utilice un lenguaje que garantice este comportamiento (que incluye la gran mayoría de los lenguajes modernos).

Su ejemplo de código es bastante claro y, de hecho, esa es a menudo la mejor manera de escribirlo. Las alternativas (como las ifdeclaraciones anidadas ) serían más desordenadas, más complicadas y, por lo tanto, más difíciles de entender.

Un par de advertencias:

Use el sentido común con respecto a la complejidad y la legibilidad.

La siguiente no es una buena idea:

if ((text_string != null && replace(text_string,"$")) || (text_string = get_new_string()) != null)

Al igual que muchas formas "inteligentes" de reducir las líneas en su código, la dependencia excesiva de esta técnica puede resultar en un código difícil de entender y difícil de entender.

Si necesita manejar errores, a menudo hay una mejor manera

Su ejemplo está bien siempre que sea un estado de programa normal para que el teléfono inteligente sea nulo. Sin embargo, si esto es un error, debe incorporar un manejo de errores más inteligente.

Evite verificaciones repetidas de la misma condición

Como señala Neil, muchas comprobaciones repetidas de la misma variable en diferentes condicionales son probablemente evidencia de una falla de diseño. Si tiene diez declaraciones diferentes esparcidas a través de su código que no deberían ejecutarse si smartphonees así null, considere si hay una mejor manera de manejar esto que verificar la variable cada vez.

Esto no se trata realmente de cortocircuito específicamente; Es un problema más general de código repetitivo. Sin embargo, vale la pena mencionarlo, porque es bastante común ver código que tiene muchas declaraciones repetidas como su declaración de ejemplo.

Digamos que estaba usando un lenguaje de estilo C sin &&y que necesitaba hacer el código equivalente como en su pregunta.

Tu código sería:

if(smartphone != null)
{
  if(smartphone.GetSignal() > 50)
  {
    // Do stuff
  }
}

Este patrón aparecería mucho.

Ahora imagine la versión 2.0 de nuestro hipotético lenguaje introductorio &&. ¡Piensa lo genial que pensarías que era!

&&es el medio reconocido e idiomático de hacer lo que hace mi ejemplo anterior. No es una mala práctica usarlo, de hecho es una mala práctica no usarlo en casos como los anteriores: un codificador experimentado en ese idioma se estaría preguntando dónde elseestaba u otra razón para no hacer las cosas de la manera normal.

El lenguaje es inteligente porque sabe que si la primera instrucción es falsa, entonces no tiene sentido evaluar la segunda instrucción y, por lo tanto, nunca se produce una excepción de referencia nula.

No, eres inteligente, porque sabías que si la primera declaración era falsa, entonces no tiene sentido siquiera evaluar la segunda declaración. El lenguaje es tonto como una caja de rocas e hizo lo que le dijiste que hiciera. (John McCarthy fue aún más inteligente y se dio cuenta de que la evaluación de cortocircuitos sería algo útil para los idiomas).

La diferencia entre ser inteligente y el lenguaje inteligente es importante, porque las buenas y malas prácticas a menudo se reducen a ser tan inteligente como sea necesario, pero no más inteligente.

Considerar:

if(smartphone != null && smartphone.GetSignal() > ((range = (range != null ? range : GetRange()) != null && range.Valid ? range.MinSignal : 50)

Esto extiende su código para verificar a range. Si el rangevalor es nulo, intenta establecerlo mediante una llamada a, GetRange()aunque eso podría fallar, por lo rangeque aún podría ser nulo. Si el rango no es nulo después de esto, y es Validentonces MinSignal, se usa su propiedad; de lo contrario, se usa un valor predeterminado de 50.

Esto también depende de &&eso, pero ponerlo en una línea probablemente sea demasiado inteligente (no estoy 100% seguro de tenerlo bien, y no voy a volver a verificarlo porque ese hecho demuestra mi punto).

Sin &&embargo, no es ese el problema aquí, pero la capacidad de usarlo para poner mucho en una expresión (algo bueno) aumenta nuestra capacidad de escribir innecesariamente expresiones difíciles de entender (algo malo).

También:

if(smartphone != null && (smartphone.GetSignal() == 2 || smartphone.GetSignal() == 5 || smartphone.GetSignal() == 8 || smartPhone.GetSignal() == 34))
{
  // do something
}

Aquí estoy combinando el uso de &&con una verificación de ciertos valores. Esto no es realista en el caso de las señales telefónicas, pero aparece en otros casos. Aquí, es un ejemplo de que no soy lo suficientemente inteligente. Si hubiera hecho lo siguiente:

if(smartphone != null)
{
  switch (smartphone.GetSignal())
  {
    case 2: case 5: case 8: case 34:
      // Do something
      break;
  }
}

GetSignal()Habría ganado tanto en legibilidad como también en rendimiento (las llamadas múltiples probablemente no se habrían optimizado).

Aquí nuevamente el problema no es tanto &&como tomar ese martillo en particular y ver todo lo demás como un clavo; no usarlo, déjame hacer algo mejor que usarlo.

Un caso final que se aleja de las mejores prácticas es:

if(a && b)
{
  //do something
}

Comparado con:

if(a & b)
{
  //do something
}

El argumento clásico de por qué podríamos favorecer a este último es que hay algún efecto secundario al evaluar bque queremos si aes verdad o no. No estoy de acuerdo con eso: si ese efecto secundario es tan importante, entonces haga que suceda en una línea de código separada.

Sin embargo, en términos de eficiencia, es probable que cualquiera de los dos sea mejor. El primero obviamente ejecutará menos código (sin evaluar ben absoluto en una ruta de código) lo que puede ahorrarnos la cantidad de tiempo que se necesita para evaluar b.

El primero, aunque también tiene una rama más. Considere si lo reescribimos en nuestro hipotético lenguaje de estilo C sin &&:

if(a)
{
  if(b)
  {
    // Do something
  }
}

Ese extra ifestá oculto en nuestro uso de &&, pero todavía está allí. Y como tal, es un caso en el que se está produciendo una predicción de rama y, por lo tanto, potencialmente una predicción errónea de rama.

Por esa razón, el if(a & b)código puede ser más eficiente en algunos casos.

Aquí diría que if(a && b)sigue siendo el enfoque de mejores prácticas para comenzar: más común, el único viable en algunos casos (donde se bproducirá un error si aes falso) y más rápido que nunca. Sin if(a & b)embargo , vale la pena señalar que a menudo es una optimización útil en algunos casos.

Puede llevar esto más lejos, y en algunos idiomas es la forma idiomática de hacer las cosas: también puede usar la evualización de cortocircuito fuera de las declaraciones condicionales, y se convierten en una forma de declaración condicional. Por ejemplo, en Perl, es idiomático que las funciones devuelvan algo falso en caso de fracaso (y algo verdadero en caso de éxito), y algo así como

open($handle, "<", "filename.txt") or die "Couldn't open file!";

es idiomático opendevuelve algo distinto de cero en el éxito (para que la dieparte posterior ornunca suceda), pero no está definido en el fracaso, y luego la llamada a diesuceder (esa es la forma en que Perl genera una excepción).

Eso está bien en los idiomas donde todos lo hacen.

Aunque generalmente estoy de acuerdo con la respuesta de dan1111 , hay un caso particularmente importante que no cubre: el caso en el que smartphonese usa simultáneamente. En ese escenario, este tipo de patrón es una fuente bien conocida de errores difíciles de encontrar.

El problema es que la evaluación de cortocircuito no es atómica. Su hilo puede comprobar que smartphonees null, puede entrar otro hilo y anularlo, y luego su hilo intenta rápidamente GetSignal()y explota.

Pero, por supuesto, sólo se hace cuando las estrellas se alinean y el tiempo sus hilos es simplemente correcto.

Entonces, si bien este tipo de código es muy común y útil, es importante conocer esta advertencia para que pueda prevenir con precisión este error desagradable.

Hay muchas situaciones en las que quiero verificar una condición primero, y solo quiero verificar una segunda condición si la primera condición tuvo éxito. A veces, simplemente por eficiencia (porque no tiene sentido verificar la segunda condición si la primera ya falló), a veces porque de lo contrario mi programa se bloquearía (su verificación de NULL primero), a veces porque de lo contrario los resultados serían terribles. (SI (quiero imprimir un documento) Y (falló la impresión de un documento)) LUEGO muestra un mensaje de error: no desea imprimir el documento y verifique si la impresión falló si no desea imprimir un documento en el ¡primer lugar!

Por lo tanto, había una GRAN necesidad de garantizar la evaluación de cortocircuito de las expresiones lógicas. No estaba presente en Pascal (que tenía una evaluación de cortocircuito no garantizada), lo cual fue un gran dolor; Lo vi por primera vez en Ada y C.

Si realmente cree que esto es una "mala práctica", tome cualquier código de código moderadamente complejo, vuelva a escribirlo para que funcione bien sin una evaluación de corto circuito, y vuelva y díganos cómo le gustó. Es como decirle que respirar produce dióxido de carbono y preguntar si respirar es una mala práctica. Intente sin ella por unos minutos.

Una consideración, no mencionada en otras respuestas: a veces tener estas comprobaciones puede sugerir una posible refactorización del patrón de diseño de Objetos nulos . Por ejemplo:

if (currentUser && currentUser.isAdministrator()) 
  doSomething();

Podría simplificarse para ser simplemente:

if (currentUser.isAdministrator())
  doSomething ();

Si currentUserestá predeterminado en algún 'usuario anónimo' o 'usuario nulo' con una implementación alternativa si el usuario no ha iniciado sesión.

No siempre es un olor a código, sino algo a tener en cuenta.

Voy a ser impopular y decir Sí, es una mala práctica. SI la funcionalidad de su código depende de él para implementar la lógica condicional.

es decir

 if(quickLogicA && slowLogicB) {doSomething();}

es bueno pero

if(logicA && doSomething()) {andAlsoDoSomethingElse();}

es malo, y seguramente nadie estaría de acuerdo?

if(doSomething() || somethingElseIfItFailed() && anotherThingIfEitherWorked() & andAlwaysThisThing())
{/* do nothing */}

Razonamiento:

Sus errores de código si ambos lados son evaluados en lugar de simplemente no realizar la lógica. Se ha argumentado que esto no es un problema, el operador 'y' se define en c #, por lo que el significado es claro. Sin embargo, sostengo que esto no es cierto.

Motivo 1. Histórico

Esencialmente, usted está confiando (lo que originalmente se pensó como) una optimización del compilador para proporcionar funcionalidad en su código.

Aunque en c # la especificación del lenguaje garantiza que el operador booleano 'y' haga un cortocircuito. Esto no es cierto para todos los idiomas y compiladores.

wikipeda enumera varios idiomas y su opinión sobre el cortocircuito http://en.wikipedia.org/wiki/Short-circuit_evaluation, ya que puede, hay muchos enfoques. Y un par de problemas adicionales que no voy a abordar aquí.

En particular, FORTRAN, Pascal y Delphi tienen indicadores de compilación que alternan este comportamiento.

Por lo tanto: puede encontrar que su código funciona cuando se compila para su lanzamiento, pero no para la depuración o con un compilador alternativo, etc.

Motivo 2. Diferencias lingüísticas

Como puede ver en wikipedia, no todos los idiomas adoptan el mismo enfoque para cortocircuitos, incluso cuando especifican cómo deben manejarlo los operadores booleanos.

En particular, el operador 'y' de VB.Net no produce cortocircuitos y TSQL tiene algunas peculiaridades.

Dado que es probable que una 'solución' moderna incluya varios idiomas, como javascript, regex, xpath, etc., debe tener esto en cuenta al aclarar la funcionalidad de su código.

Razón 3. Diferencias dentro de un idioma.

Por ejemplo, el if en línea de VB se comporta de manera diferente a su if. Una vez más, en las aplicaciones modernas, su código puede moverse, por ejemplo, el código detrás y un formulario web en línea, debe tener en cuenta las diferencias de significado.

Motivo 4. Su ejemplo específico de comprobación nula del parámetro de una función

Este es un muy buen ejemplo. En eso es algo que todos hacen, pero en realidad es una mala práctica cuando lo piensas.

Es muy común ver este tipo de verificación, ya que reduce sus líneas de código considerablemente. Dudo que alguien lo mencione en una revisión de código. (¡y obviamente de los comentarios y la calificación puedes ver que es popular!)

Sin embargo, falla en las mejores prácticas por más de una razón.

1. Está muy claro por numerosas fuentes, que la mejor práctica es verificar la validez de sus parámetros antes de usarlos y lanzar una excepción si no son válidos. Su fragmento de código no muestra el código circundante, pero probablemente debería estar haciendo algo como:

   if (smartphone == null)
   {
       //throw an exception
   }
   // perform functionality
   

2. Está llamando a una función desde dentro de la declaración condicional. Aunque el nombre de su función implica que simplemente calcula un valor, puede ocultar los efectos secundarios. p.ej

   Smartphone.GetSignal() { this.signal++; return this.signal; }
   
   else if (smartphone.GetSignal() < 1)
   {
       // Do stuff 
   }
   else if (smartphone.GetSignal() < 2)
   {
       // Do stuff 
   }
   

   Las mejores prácticas sugerirían:

   var s = smartphone.GetSignal();
   if(s < 50)
   {
       //do something
   }
   

Si aplica estas mejores prácticas a su código, puede ver que su oportunidad de lograr un código más corto mediante el uso del condicional oculto desaparece. La mejor práctica es hacer algo , manejar el caso donde el teléfono inteligente es nulo.

Creo que encontrará que este también es el caso para otros usos comunes. Hay que recordar que también tenemos:

condicionales en línea

var s = smartphone!=null ? smartphone.GetSignal() : 0;

y nula coalescencia

var s = smartphoneConnectionStatus ?? "No Connection";

que proporcionan una funcionalidad similar de longitud de código corto con más claridad

Numerosos enlaces para apoyar todos mis puntos:

https://stackoverflow.com/questions/1158614/what-is-the-best-practice-in-case-one-argument-is-null

Validación de parámetros de constructor en C # - Mejores prácticas

¿Se debe verificar si es nulo si no espera que sea nulo?

https://msdn.microsoft.com/en-us/library/seyhszts%28v=vs.110%29.aspx

https://stackoverflow.com/questions/1445867/why-would-a-language-not-use-short-circuit-evaluation

http://orcharddojo.net/orchard-resources/Library/DevelopmentGuidelines/BestPractices/CSharp

Ejemplos de indicadores del compilador que afectan el cortocircuito:

Fortran: "sce | nosce De manera predeterminada, el compilador realiza una evaluación de cortocircuito en expresiones lógicas seleccionadas utilizando las reglas de XL Fortran. Especificar sce permite que el compilador use reglas de Fortran no XL. El compilador realizará una evaluación de cortocircuito si las reglas actuales lo permiten . El valor predeterminado es nosce ".

Pascal: "B - Una directiva de bandera que controla la evaluación de cortocircuito. Consulte el Orden de evaluación de operandos de operadores diádicos para obtener más información sobre la evaluación de cortocircuito. Consulte también la opción del compilador -sc para el compilador de línea de comandos para obtener más información ( documentado en el Manual del usuario) ".

Delphi: "Delphi admite la evaluación de cortocircuito de forma predeterminada. Se puede desactivar con la directiva del compilador {$ BOOLEVAL OFF}".