Comment prévenir la pointe de la flèche anti-modèle

Question

Je suis un peu confus sur la façon de mieux refactoriser mon code en quelque chose de plus lisible.

Considérer ce morceau de code:

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;

Comme vous pouvez le voir, beaucoup de imbriquée if blocs, nécessaire parce que chaque si imbriquées dépend de la valeur précédente.

Maintenant, je me demandais comment faire pour que mon code un peu plus propre à cet égard.

Quelques options que j'ai pensé à moi serait:

• de retour après chaque clause if, sens j'aurais plusieurs endroits où je laisse ma méthode
• jeter ArgumentNullExceptions, après quoi j'avais attraper à la fin et le lieu de l'instruction de retour dans ma clause finally (ou à l'extérieur du bloc try/catch)
• travailler avec un label et d' goto:

La plupart de ces options semblent un peu "sale" pour moi, donc je me demandais si il y avait un bon moyen de nettoyer ce gâchis que j'ai créé.

Answer 1

J'irais pour le multiple, return des déclarations. Cela rend le code plus facile à lire et à comprendre.

Ne pas utiliser goto pour des raisons évidentes.

N'utilisez pas des exceptions, car le chèque que vous faites n'est pas exceptionnelle, c'est quelque chose que vous pouvez attendre de sorte que vous devez juste tenir compte. Programmation à l'encontre des exceptions est aussi un anti-pattern.

Answer 2

Envisager l'inversion de l'null chèques à:

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

Answer 3

Vous pouvez la chaîne d'expressions. Une affectation renvoie la valeur affectée, de sorte que vous pouvez vérifier le résultat. Aussi, vous pouvez utiliser la variable attribuée dans l'expression suivante.

Dès qu'une expression renvoie la valeur false, les autres ne sont pas plus exécutée, parce que l'ensemble de l'expression return false déjà (en raison de l' and de l'opération).

Donc, quelque chose comme cela devrait fonctionner:

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 4

C'est l'un des rares scénarios où il est parfaitement acceptable (si pas souhaitable) d'utiliser des goto.

Dans des fonctions comme cela, il y aura souvent des ressources qui sont allouées ou de l'état des modifications qui sont apportées à mi-parcours qui doivent être effacés avant que la fonction se termine.

Le problème habituel de retour en fonction des solutions (par exemple, rexcfnghk et Gerrie Schenck), c'est que vous devez vous rappeler pour annuler les modifications de l'état avant chaque retour. Cela conduit à la duplication de code et ouvre la porte à de subtiles erreurs, en particulier dans les plus grandes fonctions. Ne pas le faire.

---

CERT effectivement recommande une approche structurelle basée sur goto.

En particulier, la note de leur exemple de code d' copy_process en kernel/fork.c du noyau Linux. Une version simplifiée de la notion est comme suit:

    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;

Essentiellement, c'est juste une version plus lisible de la "pointe de flèche" du code qui n'utilise pas inutile d'indentation ou duplication de code. Ce concept peut être facilement étendue à ce qui convient le mieux à votre situation.

---

Comme note finale, en particulier concernant CERT premier exemple conforme, je veux juste ajouter que, chaque fois que possible, il est plus simple pour la conception de votre code afin que le nettoyage peut être manipulé toutes à la fois. De cette façon, vous pouvez écrire du code comme ceci:

    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 5

D'abord votre suggestion (de retour après chaque clause if) est un bon moyen de sortir:

  // 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 deuxième possibilité (dans votre cas) est de modifier légèrement votre getbar() ainsi que getmoo() les fonctions telles qu'elles retournent null on null input, de sorte que vous aurez

  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 troisième possibilité est que dans les cas compliqués, vous pouvez utiliser l'Objet Null Desing Patteren

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