¿Es una mala práctica utilizar return dentro de un método vacío?

92

Imagina el siguiente código:

void DoThis()
{
    if (!isValid) return;

    DoThat();
}

void DoThat() {
    Console.WriteLine("DoThat()");
}

¿Está bien usar una devolución dentro de un método vacío? ¿Tiene alguna penalización por desempeño? O sería mejor escribir un código como este:

void DoThis()
{
    if (isValid)
    {
        DoThat();
    }
}

fuente
1
¿Qué pasa con: void DoThis () {if (isValid) DoThat (); }
Dscoduc
30
imagina el código? ¿Por qué? ¡Está justo ahí! :-D
STW
Esta es una buena pregunta, siempre creo que es una buena práctica usar return; para salir del método o función. Especialmente en un método de minería de datos LINQ que tiene varios resultados IQueryable <T> y todos dependen unos de otros. Si alguno de ellos no da resultado, alertar y salir.
Cheung

Respuestas:

173

Un método de retorno en un vacío no está mal, es una práctica común invertir ifdeclaraciones para reducir el anidamiento .

Y tener menos anidamiento en sus métodos mejora la legibilidad y el mantenimiento del código.

En realidad, si tiene un método void sin ninguna declaración de retorno, el compilador siempre generará una instrucción ret al final.

Christian C. Salvadó
fuente
33

Existe otra gran razón para usar guardias (a diferencia del código anidado): si otro programador agrega código a su función, está trabajando en un entorno más seguro.

Considerar:

void MyFunc(object obj)
{
    if (obj != null)
    {
        obj.DoSomething();
    }
}

versus:

void MyFunc(object obj)
{
    if (obj == null)
        return;

    obj.DoSomething();
}

Ahora, imagina que otro programador agrega la línea: obj.DoSomethingElse ();

void MyFunc(object obj)
{
    if (obj != null)
    {
        obj.DoSomething();
    }

    obj.DoSomethingElse();
}

void MyFunc(object obj)
{
    if (obj == null)
        return;

    obj.DoSomething();
    obj.DoSomethingElse();
}

Obviamente, este es un caso simplista, pero el programador ha agregado un bloqueo al programa en la primera instancia (código anidado). En el segundo ejemplo (salida anticipada con guardias), una vez que pasa la guardia, su código está a salvo del uso no intencional de una referencia nula.

Claro, un gran programador no comete errores como este (a menudo). Pero es mejor prevenir que curar: podemos escribir el código de una manera que elimine por completo esta fuente potencial de errores. El anidamiento agrega complejidad, por lo que las mejores prácticas recomiendan refactorizar el código para reducir el anidamiento.

Jason Williams
fuente
Sí, pero por otro lado, varias capas de anidamiento, con sus condiciones, hacen que el código sea aún más propenso a errores, la lógica más difícil de rastrear y, lo que es más importante, más difícil de depurar. Las funciones planas son un mal menor, en mi opinión.
Skrim
18
¡Estoy argumentando a favor de una reducción de la anidación! :-)
Jason Williams
Estoy de acuerdo con ésto. Además, desde el punto de vista de la refactorización, es más fácil y seguro refactorizar el método si obj se convierte en una estructura o algo que pueda garantizar que no será nulo.
Phil Cooper
18

¿¿¿Mala práctica??? De ninguna manera. De hecho, siempre es mejor manejar las validaciones volviendo del método lo antes posible si las validaciones fallan. De lo contrario, resultaría en una gran cantidad de ifs y elses anidados. Terminar temprano mejora la legibilidad del código.

También verifique las respuestas a una pregunta similar: ¿Debería usar la declaración return / continue en lugar de if-else?

SO Usuario
fuente
8

No es una mala práctica (por todas las razones ya mencionadas). Sin embargo, cuantos más retornos tenga un método, más probable será que se divida en métodos lógicos más pequeños.

Mike Hall
fuente
8

El primer ejemplo está usando una declaración de guardia. De Wikipedia :

En programación de computadoras, una guardia es una expresión booleana que debe evaluarse como verdadera si la ejecución del programa continuará en la rama en cuestión.

Creo que tener un montón de guardias en la parte superior de un método es una forma perfectamente comprensible de programar. Básicamente está diciendo "no ejecute este método si alguno de estos es verdadero".

Entonces, en general, le gustaría esto:

void DoThis()
{
  if (guard1) return;
  if (guard2) return;
  ...
  if (guardN) return;

  DoThat();
}

Creo que eso es mucho más legible entonces:

void DoThis()
{
  if (guard1 && guard2 && guard3)
  {
    DoThat();
  }
}
cdmckay
fuente
3

No hay ninguna penalización de rendimiento, sin embargo, la segunda parte del código es más legible y, por lo tanto, más fácil de mantener.

Russell
fuente
Russell, no estoy de acuerdo con tu opinión, pero no debería haber sido rechazado por ella. +1 para igualarlo. Por cierto, creo que una prueba booleana y un retorno en una sola línea seguida de una línea en blanco es una clara indicación de lo que está sucediendo. por ejemplo, el primer ejemplo de Rodrigo.
Paul Sasik
No estoy de acuerdo con esto. Aumentar el anidamiento no mejora la legibilidad. El primer fragmento de código está utilizando una declaración de "guardia", que es un patrón perfectamente comprensible.
cdmckay
Yo también estoy en desacuerdo. Las cláusulas de protección que abandonan una función antes de tiempo generalmente se consideran algo bueno hoy en día para ayudar al lector a comprender la implementación.
Pete Hodgson
2

En este caso, su segundo ejemplo es un código mejor, pero eso no tiene nada que ver con regresar de una función void, es simplemente porque el segundo código es más directo. Pero regresar de una función vacía está completamente bien.

Imagist
fuente
0

Está perfectamente bien y sin "penalización de rendimiento", pero nunca escriba una declaración "si" sin corchetes.

Siempre

if( foo ){
    return;
}

Es mucho más legible; y nunca asumirá accidentalmente que algunas partes del código están dentro de esa declaración cuando no lo están.

Seda del mediodía
fuente
2
legible es subjetivo. en mi humilde opinión, cualquier cosa que se agregue al código que sea innecesaria lo hace menos legible ... (tengo que leer más, y luego me pregunto por qué está allí y perder el tiempo tratando de asegurarme de que no me falta algo) ... pero esa es mi opinión subjetiva
Charles Bretana
10
La mejor razón para incluir siempre las llaves es menos sobre legibilidad y más sobre seguridad. Sin las llaves, es muy fácil para alguien corregir un error que requiera declaraciones adicionales como parte del si, no prestar suficiente atención y agregarlas sin agregar también las llaves. Al incluir siempre los aparatos ortopédicos, se elimina este riesgo.
Scott Dorman
2
Sedoso, presione enter antes de su {. Esto lo alinea {con su }en la misma columna, lo que ayuda en gran medida a la legibilidad (es mucho más fácil encontrar las llaves de apertura / cierre correspondientes).
Imagist
1
@Imagist Dejaré eso a mis preferencias personales; y está hecho de la manera que prefiero :)
Noon Silk
1
Si cada llave cerrada se empareja con una llave abierta que se coloca en el mismo nivel de sangría , entonces ifserá fácil distinguir visualmente qué declaraciones requieren llaves cerradas y, por lo tanto, tener ifdeclaraciones que controlen una sola declaración será seguro. Empujar la abrazadera abierta de nuevo a la línea con el ifguarda una línea de espacio vertical en cada declaración múltiple if, pero requerirá el uso de una línea de abrazadera cerrada que de otro modo sería innecesaria.
supercat
0

Voy a estar en desacuerdo con todos ustedes, jóvenes secuestradores en este caso.

Usar el retorno en medio de un método, nulo o no, es una muy mala práctica, por razones que fueron articuladas con bastante claridad, hace casi cuarenta años, por el difunto Edsger W. Dijkstra, comenzando con la conocida "Declaración GOTO considerada perjudicial ", y continuando en" Programación estructurada ", de Dahl, Dijkstra y Hoare.

La regla básica es que cada estructura de control y cada módulo debe tener exactamente una entrada y una salida. Un retorno explícito en el medio del módulo rompe esa regla y hace que sea mucho más difícil razonar sobre el estado del programa, lo que a su vez hace que sea mucho más difícil decir si el programa es correcto o no (que es una propiedad mucho más fuerte que "si parece funcionar o no").

La "Declaración de GOTO considerada perjudicial" y la "Programación estructurada" dieron inicio a la revolución de la "Programación estructurada" de la década de 1970. Esas dos piezas son las razones por las que tenemos if-then-else, while-do y otras construcciones de control explícitas en la actualidad, y por qué las declaraciones GOTO en lenguajes de alto nivel están en la lista de especies en peligro de extinción. (Mi opinión personal es que deben estar en la lista de especies extintas).

Vale la pena señalar que Message Flow Modulator, la primera pieza de software militar que NUNCA pasó las pruebas de aceptación en el primer intento, sin desviaciones, exenciones o palabrería de "sí, pero", se escribió en un idioma que ni siquiera tenía una declaración GOTO.

También vale la pena mencionar que Nicklaus Wirth cambió la semántica de la declaración RETURN en Oberon-07, la última versión del lenguaje de programación Oberon, convirtiéndola en una parte final de la declaración de un procedimiento escrito (es decir, una función), en lugar de una instrucción ejecutable en el cuerpo de la función. Su explicación del cambio decía que lo hizo precisamente porque la forma anterior ERA una violación del principio de una salida de la Programación Estructurada.

John R. Strohm
fuente
2
@John: superamos el mandato judicial de Dykstra sobre devoluciones múltiples casi cuando superamos a Pascal (la mayoría de nosotros, de todos modos).
John Saunders
Los casos en los que se requieren múltiples devoluciones son con frecuencia signos de que un método está intentando hacer demasiado y debe reducirse. No voy a ir tan lejos como John con esto, y una declaración de retorno como parte de la validación de parámetros puede ser una excepción razonable, pero entiendo de dónde viene la idea.
Kyoryu
@nairdaen: Todavía existe controversia sobre las excepciones en ese trimestre. Mi pauta es la siguiente: si el sistema en desarrollo DEBE solucionar el problema que causó la condición excepcional original y no me importa molestar al tipo que tendrá que escribir ese código, lanzaré una excepción. Luego me gritaron en una reunión, porque el tipo no se molestó en detectar la excepción y la aplicación se bloqueó durante la prueba, y le explico POR QUÉ tiene que solucionar el problema y las cosas se calman nuevamente.
John R. Strohm
Hay una gran diferencia entre las declaraciones de guardia y los gotos. La maldad de los gotos es que pueden saltar a cualquier parte, por lo que puede ser muy confuso desentrañar y recordar. Las declaraciones de protección son exactamente lo opuesto: proporcionan una entrada cerrada a un método, después de lo cual sabe que está trabajando en un entorno "seguro", lo que reduce la cantidad de cosas que debe considerar mientras escribe el resto del código (por ejemplo, "Sé que este puntero nunca será nulo, por lo que no necesito manejar ese caso en todo el código").
Jason Williams
@Jason: La pregunta original no se refería específicamente a declaraciones de guardia, sino a declaraciones de retorno aleatorias en medio de un método. El ejemplo dado parece ser un guardia. La cuestión clave es que, en el sitio de devolución, desea poder razonar sobre lo que hizo o no hizo el método, y las devoluciones aleatorias lo hacen más difícil, por EXACTAMENTE las mismas razones por las que los GOTO aleatorios lo hacen más difícil. Ver: Dijkstra, "Declaración de GOTO considerada perjudicial". En cuanto a la sintaxis, cdmckay dio, en otra respuesta, su sintaxis preferida para los guardias; No estoy de acuerdo con su opinión sobre qué forma es más legible.
John R. Strohm
0

Mientras usa protectores, asegúrese de seguir ciertas pautas para no confundir a los lectores.

  • la función hace una cosa
  • los guardias solo se introducen como la primera lógica en la función
  • la parte no anidada contiene la intención principal de la función

Ejemplo

// guards point you to the core intent
void Remove(RayCastResult rayHit){

  if(rayHit== RayCastResult.Empty)
    return
    ;
  rayHit.Collider.Parent.Remove();
}

// no guards needed: function split into multiple cases
int WonOrLostMoney(int flaw)=>
  flaw==0 ? 100 :
  flaw<10 ? 30 :
  flaw<20 ? 0 :
  -20
;
Orangle
fuente
-3

Lanza una excepción en lugar de no devolver nada cuando el objeto es nulo, etc.

Su método espera que el objeto no sea nulo y no es el caso, por lo que debe lanzar una excepción y dejar que la persona que llama lo maneje.

Pero el regreso temprano no es una mala práctica de otra manera.

Dhananjay
fuente
1
La respuesta no responde a la pregunta. La pregunta es un método nulo, por lo que no se devuelve nada. Además, los métodos no tienen parámetros. Tengo el punto de no devolver nulo si el tipo de retorno es un objeto, pero eso no se aplica a esta pregunta.
Luke Hammer