c# - patterns - Cómo prevenir el antipatrón de punta de flecha

lista de antipatrones

Question

Estoy un poco confundido sobre cómo refactorizar mejor mi código en algo más legible.

Considera este pedazo de código:

var foo = getfoo();
if(foo!=null)
{
    var bar = getbar(foo);
    if(bar!=null)
    {
        var moo = getmoo(bar);
        if(moo!=null)
        {
            var cow = getcow(moo);
            ...
        }
    }
}
return;

Como puede ver, se necesitan muchos bloques anidados if , porque cada uno anidado depende de los valores anteriores.

Ahora me preguntaba cómo hacer que mi código sea un poco más limpio en este sentido.

Algunas de las opciones en las que he pensado serían:

• regresar después de cada cláusula if, lo que significa que tendría varios lugares donde dejo mi método
• lanzar ArgumentNullException s, después de lo cual los vería al final y colocaría la declaración return en mi cláusula finally (o fuera del bloque try / catch)
• trabajar con una etiqueta y goto:

La mayoría de estas opciones me parecen un poco "sucias", así que me preguntaba si había una buena manera de solucionar este problema que he creado.

Answer 1

Este es uno de los pocos escenarios donde es perfectamente aceptable (si no deseable) utilizar goto .

En funciones como esta, a menudo habrá recursos asignados o cambios de estado que se realizan a mitad de camino que deben deshacerse antes de que la función finalice.

El problema habitual con las soluciones basadas en el retorno (por ejemplo, rexcfnghk y Gerrie Schenck) es que debe recordar deshacer los cambios de estado antes de cada devolución . Esto conduce a la duplicación de código y abre la puerta a errores sutiles, especialmente en funciones más grandes. No hagas esto.

CERT realmente recomienda un enfoque estructural basado en goto .

En particular, tenga en cuenta su código de ejemplo tomado de copy_process en kernel/fork.c del kernel de Linux. Una versión simplificada del concepto es la siguiente:

    if (!modify_state1(true))
        goto cleanup_none;
    if (!modify_state2(true))
        goto cleanup_state1;
    if (!modify_state3(true))
        goto cleanup_state2;

    // ...

cleanup_state3:
    modify_state3(false);
cleanup_state2:
    modify_state2(false);
cleanup_state1:
    modify_state1(false);
cleanup_none:
    return;

Esencialmente, esta es solo una versión más legible del código "punta de flecha" que no usa indentación innecesaria o código duplicado. Este concepto puede extenderse fácilmente a lo que mejor se adapte a su situación.

Como nota final, especialmente con respecto al primer ejemplo compatible de CERT, solo quiero agregar que, siempre que sea posible, es más sencillo diseñar su código para que la limpieza pueda ser manejada de una vez. De esa forma, puedes escribir código como este:

    FILE *f1 = null;
    FILE *f2 = null;
    void *mem = null;

    if ((f1 = fopen(FILE1, "r")) == null)
        goto cleanup;
    if ((f2 = fopen(FILE2, "r")) == null)
        goto cleanup;
    if ((mem = malloc(OBJSIZE)) == null)
        goto cleanup;

    // ...

cleanup:
    free(mem); // These functions gracefully exit given null input
    close(f2);
    close(f1);
    return;

Answer 2

Considere la posibilidad de invertir las comprobaciones nulas en:

var foo = getfoo();
if (foo == null)
{
    return;
}
var bar = getbar(foo);
if (bar == null)
{
    return;
}
...etc

Answer 3

Extraño, nadie mencionó el método de encadenamiento.

Si creas una vez un método encadenando clase

Public Class Chainer(Of R)

    Public ReadOnly Result As R

    Private Sub New(Result As R)
        Me.Result = Result
    End Sub

    Public Shared Function Create() As Chainer(Of R)
        Return New Chainer(Of R)(Nothing)
    End Function

    Public Function Chain(Of S)(Method As Func(Of S)) As Chainer(Of S)
        Return New Chainer(Of S)(Method())
    End Function

    Public Function Chain(Of S)(Method As Func(Of R, S)) As Chainer(Of S)
        Return New Chainer(Of S)(If(Result Is Nothing, Nothing, Method(Result)))
    End Function
End Class

Puede usarlo en todas partes para componer cualquier número de funciones en una secuencia de ejecución para producir un resultado o una nada (nulo)

Dim Cow = Chainer(Of Object).Create.
    Chain(Function() GetFoo()).
    Chain(Function(Foo) GetBar(Foo)).
    Chain(Function(Bar) GetMoo(Bar)).
    Chain(Function(Moo) GetCow(Moo)).
    Result

Answer 4

Hazlo en la vieja escuela

var foo;
var bar;
var moo;
var cow;
var failed = false;
failed = failed || (foo = getfoo()) == null;
failed = failed || (bar = getbar(foo)) == null;
failed = failed || (moo = getmoo(bar)) == null;
failed = failed || (cow = getcow(moo)) == null;

Mucho más claro, sin flecha, y extensible para siempre.

No vayas al Dark Side y usas goto o return .

Answer 5

La respuesta de Rex Kerr es realmente muy agradable.
Sin embargo, si puedes cambiar el código, la respuesta de Jens Schauder es probablemente mejor (Patrón de Objeto Nulo)

Si puede hacer que el ejemplo sea más específico, probablemente pueda obtener aún más respuestas
Por ejemplo, dependiendo de la "ubicación" de los métodos, puede tener algo como:

namespace ConsoleApplication8
{
    using MyLibrary;
    using static MyLibrary.MyHelpers;

    class Foo { }
    class Bar { }
    class Moo { }
    class Cow { }


    internal class Program
    {
        private static void Main(string[] args)
        {
            var cow = getfoo()?.getbar()?.getmoo()?.getcow();
        }
    }
}

namespace MyLibrary
{
    using ConsoleApplication8;
    static class MyExtensions
    {
        public static Cow getcow(this Moo moo) => null;
        public static Moo getmoo(this Bar bar) => null;
        public static Bar getbar(this Foo foo) => null;
    }

    static class MyHelpers
    {
        public static Foo getfoo() => null;
    }
}

Answer 6

Me gustaría ir a las declaraciones de return múltiple. Esto hace que el código sea fácil de leer y comprender.

No use goto por razones obvias.

No use excepciones porque el control que está realizando no es excepcional, es algo que puede esperar, así que debe tenerlo en cuenta. La programación en contra de excepciones también es un antipatrón.

Answer 7

Primero, su sugerencia (retorno después de cada cláusula if) es una salida bastante buena:

  // Contract (first check all the input)
  var foo = getfoo();

  if (Object.ReferenceEquals(null, foo))
    return; // <- Or throw exception, put assert etc.

  var bar = getbar(foo);

  if (Object.ReferenceEquals(null, bar))
    return; // <- Or throw exception, put assert etc.

  var moo = getmoo(bar);

  if (Object.ReferenceEquals(null, moo))
    return; // <- Or throw exception, put assert etc.

  // Routine: all instances (foo, bar, moo) are correct (not null) and we can work with them
  ...

La segunda posibilidad (en tu caso) es modificar ligeramente tus funciones getbar () y getmoo () de manera que devuelvan nulo en la entrada nula, por lo que tendrás

  var foo = getfoo();
  var bar = getbar(foo); // return null if foo is null
  var moo = getmoo(bar); // return null if bar is null

  if ((foo == null) || (bar == null) || (moo == null))
    return; // <- Or throw exception, put assert(s) etc.    

  // Routine: all instances (foo, bar, moo) are correct (not null)
  ...

La tercera posibilidad es que, en casos complicados, se puede usar Null Object Desing Patteren

http://en.wikipedia.org/wiki/Null_Object_pattern

Answer 8

Puedes encadenar expresiones. Una asignación devuelve el valor asignado, por lo que puede verificar su resultado. Además, puede usar la variable asignada en la siguiente expresión.

Tan pronto como una expresión devuelve falso, los demás ya no se ejecutan, porque la expresión completa ya devolvería falsa (debido a la operación and ).

Entonces, algo como esto debería funcionar:

Foo foo; Bar bar; Moo moo; Cow cow;

if( (foo = getfoo()) != null &&
    (bar = getbar(foo)) != null &&
    (moo = getmoo(bar)) != null &&
    (cow = getcow(moo)) != null )
{
  ..
}

Answer 9

Si puede cambiar las cosas a las que está llamando, puede cambiarlas para que nunca vuelvan nulas, sino como http://en.wikipedia.org/wiki/Null_Object_pattern .

Esto le permitiría perder todos los ifs por completo.

Answer 10

Una alternativa es utilizar un solo bucle "falso" para controlar el flujo del programa. No puedo decir que lo recomiende, pero definitivamente es mejor y más legible que la punta de flecha.

Agregar una "etapa", "fase" o algo así como esa variable puede simplificar la depuración y / o el manejo de errores.

int stage = 0;
do { // for break only, possibly with no indent

var foo = getfoo();
if(foo==null) break;

stage = 1;
var bar = getbar(foo);
if(bar==null) break;

stage = 2;
var moo = getmoo(bar);
if(moo==null) break;

stage = 3;
var cow = getcow(moo);

return 0; // end of non-erroreous program flow
}  while (0); // make sure to leave an appropriate comment about the "fake" while

// free resources if necessary
// leave an error message
ERR("error during stage %d", stage);

//return a proper error (based on stage?)
return ERROR;

Answer 11

Este es el único caso en el que usaría goto .

Su ejemplo podría no ser suficiente para llevarme al límite, y las devoluciones múltiples son mejores si su método es lo suficientemente simple. Pero este patrón puede ser bastante extenso, y a menudo necesita un código de limpieza al final. Mientras uso la mayoría de las otras respuestas aquí si puedo, a menudo la única solución legible es usar goto .

(Cuando lo haga, asegúrese de poner todas las referencias a la etiqueta dentro de un bloque para que cualquier persona que mire el código sepa que tanto las variables de los goto como las de las variables se limitan a esa parte del código).

En Javascript y Java puedes hacer esto:

bigIf:  {

    if (!something)      break bigIf;
    if (!somethingelse)  break bigIf;
    if (!otherthing)     break bigIf;

    // Conditionally do something...
}
// Always do something else...
return;

Javascript y Java no tienen goto , lo que me lleva a creer que otras personas han notado que en esta situación los necesitas.

Una excepción también me funcionaría, a excepción del lío try / catch que fuerzas en el código de llamada. Además , C # pone un rastro de pila en el tiro, lo que ralentizará el código, particularmente si se cancela en el primer chequeo.

Answer 12

try
{
  if (getcow(getmoo(getbar(getfoo()))) == null)
  {
    throw new NullPointerException();
  }
catch(NullPointerException ex)
{
  return; //or whatever you want to do when something is null
}

//... rest of the method

Esto mantiene despejada la lógica principal del método, y tiene solo un retorno excepcional. Sus desventajas son que puede ser lento si los métodos get * son lentos y que es difícil decir en un depurador qué método devolvió el valor nulo.

Answer 13

var foo =                        getFoo();
var bar = (foo == null) ? null : getBar(foo);
var moo = (bar == null) ? null : getMoo(bar);
var cow = (moo == null) ? null : getCow(moo);
if (cow != null) {
  ...
}