Mejores prácticas sobre if / return

El ejemplo 2 se conoce como bloque de guarda. Es más adecuado para devolver / lanzar la excepción antes si algo salió mal (parámetro incorrecto o estado no válido). En el flujo lógico normal, es mejor usar el Ejemplo 1

Mi estilo personal es usar el if para los bloques de guarda y el if / else en el código de procesamiento del método real.

En este caso, estás usando myString == null como condición de guarda, por lo que tendería a usar el patrón único de if .

Considere el código que es un poco más complicado:

Ejemplo 1:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }
    else{
        //processing block
        myString = escapedString(myString);

        if (myString == "foo"){
            //some processing here
            return false;
        }
        else{
            myString = "Name " + myString;
            //other stuff
            return true;
        }
    }
}

Ejemplo 2:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }

    //processing block
    myString = escapedString(myString);

    if (myString == "foo"){
        //some processing here
        return false;
    }
    else{
        myString = "Name " + myString;
        //other stuff
        return true;
    }
}

En el Ejemplo 1, tanto la guarda como el resto del método están en la forma if / else . Compare eso con el Ejemplo 2, donde el bloque guard está en la forma única if , mientras que el resto del método usa la forma if / else . Personalmente, encuentro que el ejemplo 2 es más fácil de entender, mientras que el ejemplo 1 parece desordenado y con sangría excesiva.

Tenga en cuenta que este es un ejemplo ideado y que podría usar else if enunciados para limpiarlo, pero mi objetivo es mostrar la diferencia entre los bloques de guarda y el código de procesamiento de la función real.

Un compilador decente debería generar la misma salida para ambos de todos modos. La única razón para usar uno u otro es la preferencia personal o para ajustarse al estilo del código existente.

personalmente, prefiero el segundo método. Siento que es más corto, tiene menos sangría y es más fácil de leer.

Mi práctica personal es la siguiente:

• No me gustan las funciones con varios puntos de salida, me resultó difícil de mantener y seguir, las modificaciones de código a veces rompen la lógica interna porque es inherentemente un poco descuidada. Cuando se trata de un cálculo complejo, creo un valor de retorno al inicio y lo devuelvo al final. Esto me obliga a seguir cuidadosamente cada ruta if-else, switch, etc., establecer el valor correctamente en las ubicaciones adecuadas. También dedico un poco de tiempo a decidir si establecer un valor de retorno predeterminado o dejarlo sin inicializar al inicio. Este método también ayuda cuando cambia la lógica o el tipo de valor de retorno o el significado.

Por ejemplo:

public bool myFunction()
{
   // First parameter loading
   String myString = getString();

   // Location of "quick exits", see the second example
   // ...

   // declaration of external resources that MUST be released whatever happens
   // ...

   // the return variable (should think about giving it a default value or not) 
   // if you have no default value, declare it final! You will get compiler 
   // error when you try to set it multiple times or leave uninitialized!
   bool didSomething = false;

   try {
     if (myString != null)
     {
       myString = "Name " + myString;
       // Do something more here...

       didSomething = true;
     } else {
       // get other parameters and data
       if ( other conditions apply ) {
         // do something else
         didSomething = true;
       }
     }

     // Edit: previously forgot the most important advantage of this version
     // *** HOUSEKEEPING!!! ***

   } finally {

     // this is the common place to release all resources, reset all state variables

     // Yes, if you use try-finally, you will get here from any internal returns too.
     // As I said, it is only my taste that I like to have one, straightforward path 
     // leading here, and this works even if you don't use the try-finally version.

   }

   return didSomething;
}

• La única excepción: "salida rápida" al inicio (o en casos raros, dentro del proceso). Si la lógica de cálculo real no puede manejar una cierta combinación de parámetros de entrada y estados internos, o tiene una solución fácil sin ejecutar el algoritmo, no ayuda tener todo el código encapsulado (a veces profundo) si se trata de bloques. Este es un "estado excepcional", que no forma parte de la lógica central, por lo que debo salirme del cálculo tan pronto como lo haya detectado. En este caso, no hay otra rama, en condiciones normales, la ejecución simplemente continúa. (Por supuesto, el "estado excepcional" se expresa mejor lanzando una excepción, pero a veces es una exageración).

Por ejemplo:

public bool myFunction()
{
   String myString = getString();

   if (null == myString)
   {
     // there is nothing to do if myString is null
     return false;
   } 

   myString = "Name " + myString;
   // Do something more here...

   // not using return value variable now, because the operation is straightforward.
   // if the operation is complex, use the variable as the previous example.

   return true;
}

La regla de "una salida" también ayuda cuando el cálculo requiere recursos externos que debe liberar, o indica que debe reiniciarse antes de abandonar la función; a veces se añaden más tarde durante el desarrollo. Con múltiples salidas dentro del algoritmo, es mucho más difícil extender todas las ramas correctamente. (Y si se producen excepciones, el lanzamiento / reinicio también se debe colocar en un bloque final, para evitar efectos secundarios en casos excepcionales ...).

Su caso parece caer en la categoría de "salida rápida antes del trabajo real", y lo escribiría como su versión de Ejemplo 2.

Prefiero emplear bloques de guarda siempre que puedo por dos razones:

1. Permiten una salida rápida dada alguna condición específica.
2. Elimina la necesidad de declaraciones complejas y no necesarias si se encuentran más adelante en el código.

En general, prefiero ver los métodos en los que la funcionalidad principal del método es clara y mínima. Los bloques de guardia ayudan a hacer que esto ocurra visualmente.

Me gusta el enfoque "Fall Through":

public bool MyFunction()
{
   string myString = GetString();

   if (myString != null)
   {
     myString = "Name " + myString;
     return true;
    }
    return false;
}

La acción tiene una condición específica, cualquier otra cosa es simplemente el retorno "predeterminado".

Si tengo una sola condición, no pasaré mucho tiempo reflexionando sobre el estilo. Pero si tengo múltiples condiciones de guardia, preferiría style2

Picuture esto. Suponga que las pruebas son complejas y realmente no desea vincularlas en una sola condición de OR para evitar la complejidad:

//Style1
if (this1 != Right)
{ 
    return;
}
else if(this2 != right2)
{
    return;
}
else if(this3 != right2)
{
    return;
}
else
{
    //everything is right
    //do something
    return;
}

versus

//Style 2
if (this1 != Right)
{ 
   return;
}
if(this2 != right2)
{
    return;
}
if(this3 != right2)
{
    return;
}


//everything is right
//do something
return;

Aquí hay dos ventajas principales

1. Está separando el código en una sola función en dos bloques visualmente logcales: un bloque superior de validaciones (guardia condiciones) y un bloque inferior de código ejecutable.

2. Si tiene que agregar / eliminar una condición, reduce sus posibilidades de desordenar toda la escalera if-elseif-else.

Otra pequeña ventaja es que tienes un par de llaves menos que cuidar.

Esto debería ser una cuestión que suene mejor.

If condition
  do something
else
  do somethingelse

se expresa mejor entonces

if condition
  do something
do somethingelse

para los métodos pequeños no será una gran diferencia, pero para los métodos compuestos más grandes puede hacer que sea más difícil de entender, ya que no será lo suficientemente separado

Me parece irritante el ejemplo 1, porque la declaración de retorno faltante al final de una función de valor devuelto activa inmediatamente el mensaje "Espera, hay algo mal". Así que en casos como este, me gustaría ir con el ejemplo 2.

Sin embargo, generalmente hay más implicados, dependiendo del propósito de la función, por ejemplo. manejo de errores, registro, etc. Así que estoy con la respuesta de Lorand Kedves en esto, y generalmente tengo un punto de salida al final, incluso al costo de una variable de marca adicional. Facilita el mantenimiento y las extensiones posteriores en la mayoría de los casos.

Cuando la instrucción if siempre devuelve, no hay razón para usar otra cosa para el código restante en la función. Si lo hace, agrega líneas adicionales y sangría adicional. Agregar un código innecesario hace que sea más difícil de leer y, como todos saben, Leer código es difícil .

En tu ejemplo, el else es obviamente innecesario, PERO ...

Al hojear rápidamente las líneas de código, los ojos suelen mirar las llaves y las hendiduras para tener una idea del flujo de código; antes de que realmente se asienten en el propio código. Por lo tanto, escribir else y colocar llaves y sangría alrededor del segundo bloque de código en realidad hace que sea más rápido para el lector ver que esta es una situación "A o B", donde se ejecutará un bloque o el otro. Es decir, el lector ve las llaves y la sangría ANTES de ver el return después del inicial if .

En mi opinión, este es uno de esos casos raros en los que agregar algo redundante al código lo hace MÁS legible, no menos.

Prefiero el ejemplo 2 porque es inmediatamente obvio que la función devuelve algo . Pero incluso más que eso, preferiría volver de un lugar, como este:

public bool MyFunction()
{
    bool result = false;

    string myString = GetString();

    if (myString != nil) {
        myString = "Name " + myString;

        result = true;
    }

    return result;
}

Con este estilo puedo:

1. Vea de inmediato que estoy devolviendo algo.

2. Capture el resultado de cada invocación de la función con un solo punto de interrupción.

Mi estilo personal tiende a desaparecer

function doQuery(string) {
    if (!query(string)) {
        query("ABORT");
        return false;
    } // else
    if(!anotherquery(string) {
        query("ABORT");
        return false;
    } // else
    return true;
}

Usar las declaraciones else comentadas para indicar el flujo del programa y mantenerlo legible, pero evitando las sangrías masivas que pueden extenderse fácilmente a través de la pantalla si hay muchos pasos involucrados.

Yo escribiría

public bool SetUserId(int id)
{
   // Get user name from id
   string userName = GetNameById(id);

   if (userName != null)
   {
       // update local ID and userName
       _id = id;
       _userNameLabelText = "Name: " + userName;
   }

   return userName != null;
}

Prefiero este estilo porque es más obvio que devolvería userName != null .

Mi regla de oro es hacer un if-return sin ningún otro (principalmente por errores) o hacer una cadena completa if-else-if sobre todas las posibilidades.

De esta manera, evito tratar de ser demasiado sofisticado con mi bifurcación, ya que cada vez que termino haciendo eso, codifico la lógica de la aplicación en mis ifs, lo que los hace más difíciles de mantener (por ejemplo, si una enumeración es correcta o ERR y escribo todos mis ifs aprovechando el hecho de que! OK < - > ERR se convierte en un problema en el culo para agregar una tercera opción a la enumeración la próxima vez.)

En su caso, por ejemplo, usaría un plano si, dado que "return if null" es un patrón común y no es probable que deba preocuparse por una tercera posibilidad (además de null / no nulo) en el futuro.

Sin embargo, si la prueba fuera algo que hiciera una inspección más exhaustiva de los datos, cometería un error en su lugar, si no, en su lugar.

Creo que hay muchos escollos en el Ejemplo 2, que en el futuro podría conducir a un código no deseado. Primero, el enfoque aquí se basa en la lógica que rodea la variable 'myString'. Por lo tanto, para ser explícitos, todas las pruebas de las condiciones deben ocurrir en un bloque de código que tenga en cuenta la lógica conocida y por defecto / desconocida .

¿Qué pasaría si posteriormente se introdujera un código involuntariamente en el ejemplo 2 que alteró significativamente la salida?

   if (myString == null)
   {
      return false;
   }

   //add some v1 update code here...
   myString = "And the winner is: ";
   //add some v2 update code here...
   //buried in v2 updates the following line was added
   myString = null;
   //add some v3 update code here...
   //Well technically this should not be hit because myString = null
   //but we already passed that logic
   myString = "Name " + myString;
   // Do something more here...
   return true;

Creo que con el bloque else inmediatamente después de la comprobación de un nulo, los programadores que agregaron las mejoras a las futuras versiones agregarán toda la lógica juntos porque ahora tenemos una cadena de lógica que no fue intencional para la regla original (devolviendo si el valor es nulo).

Confío en esto fuertemente a algunas de las líneas de guía de C # en Codeplex (enlace a eso aquí: enlace ) que indican el siguiente:

"Agregue un comentario descriptivo si se supone que el bloque predeterminado (de lo contrario) está vacío. Además, si se supone que no se puede alcanzar ese bloque, lance una InvalidOperationException para detectar futuros cambios que puedan caer en los casos existentes. Esto garantiza un mejor código, porque se han pensado en todas las rutas por las que puede viajar el código. "

Creo que es una buena práctica de programación cuando se usan bloques lógicos como este para tener siempre un bloque predeterminado agregado (if-else, caso: predeterminado) para tener en cuenta todas las rutas de código explícitamente y no dejar el código abierto a consecuencias lógicas no deseadas.