Faut-il utiliser «else» dans les situations où le flux de contrôle le rend redondant?

Je tombe parfois sur du code similaire à l'exemple suivant (ce que fait exactement cette fonction sort du cadre de cette question):

function doSomething(value) {
  if (check1(value)) {
    return -1;
  }
  else if (check2(value)) {
    return value;
  }
  else {
    return false;
  }
}

Comme vous pouvez le voir, if, else ifet les elsedéclarations sont utilisées conjointement avec la returndéclaration. Cela semble assez intuitif à un observateur occasionnel, mais je pense qu'il serait plus élégant (du point de vue d'un développeur de logiciels) de supprimer le else-s et de simplifier le code comme ceci:

function doSomething(value) {
  if (check1(value)) {
    return -1;
  }
  if (check2(value)) {
    return value;
  }
  return false;
}

Cela a du sens, car tout ce qui suit une returninstruction (dans la même portée) ne sera jamais exécuté, ce qui rend le code ci-dessus sémantiquement égal au premier exemple.

Laquelle des réponses ci-dessus correspond le mieux aux bonnes pratiques de codage? Y a-t-il des inconvénients à l'une ou l'autre méthode en ce qui concerne la lisibilité du code?

Modifier: Une suggestion en double a été faite avec cette question fournie comme référence. Je crois que ma question touche à un sujet différent, car je ne demande pas à éviter les déclarations en double comme présenté dans l'autre question. Les deux questions visent à réduire les répétitions, quoique de manières légèrement différentes.

J'aime celui sans elseet voici pourquoi:

function doSomething(value) {
  //if (check1(value)) {
  //  return -1;
  //}
  if (check2(value)) {
    return value;
  }
  return false;
}

Parce que faire ça n'a rien cassé ce qu'il n'était pas censé faire.

Détestez l'interdépendance sous toutes ses formes (y compris en nommant une fonction check2()). Isolez tout ce qui peut être isolé. Parfois, vous en avez besoin, elsemais je ne vois pas cela ici.

Je pense que cela dépend de la sémantique du code. Si vos trois cas dépendent les uns des autres, indiquez-le explicitement. Cela augmente la lisibilité du code et le rend plus facile à comprendre pour n'importe qui d'autre. Exemple:

if (x < 0) {
    return false;
} else if (x > 0) {
    return true;
} else {
    throw new IllegalArgumentException();
}

Ici, vous dépendez clairement de la valeur de xdans les trois cas. Si vous omettiez le dernier else, ce serait moins clair.

Si vos cas ne dépendent pas directement les uns des autres, omettez-les:

if (x < 0) {
    return false;
}
if (y < 0) {
    return true;
}
return false;

Il est difficile de montrer cela dans un exemple simple sans contexte, mais j'espère que vous comprendrez mon point.

Je préfère la deuxième option (séparée ifsavec non else ifet précoce return) MAIS qui est aussi longue que les blocs de code sont courts .

Si les blocs de code sont longs, il vaut mieux les utiliser else if, car sinon, vous n'auriez pas une compréhension claire des différents points de sortie du code.

Par exemple:

function doSomething(value) {
  if (check1(value)) {
    // ...
    // imagine 200 lines of code
    // ...
    return -1;
  }
  if (check2(value)) {
    // ...
    // imagine 150 lines of code
    // ...
    return value;
  }
  return false;
}

Dans ce cas, il vaut mieux l'utiliser else if, stocker le code retour dans une variable et avoir une seule phrase retour à la fin.

function doSomething(value) {
  retVal=0;
  if (check1(value)) {
    // ...
    // imagine 200 lines of code
    // ...
    retVal=-1;
  } else if (check2(value)) {
    // ...
    // imagne 150 lines of code
    retVal=value;
  }
  return retVal;
}

Mais comme il faut s'efforcer que les fonctions soient courtes, je dirais de rester court et de quitter tôt comme dans votre premier extrait de code.

J'utilise les deux dans des circonstances différentes. Lors d'un contrôle de validation, j'omet le reste. Dans un flux de contrôle, j'utilise le reste.

bool doWork(string name) {
  if(name.empty()) {
    return false;
  }

  //...do work
  return true;
}

contre

bool doWork(int value) {
  if(value % 2 == 0) {
    doWorkWithEvenNumber(value);
  }
  else {
    doWorkWithOddNumber(value);
  }
}

Le premier cas se lit plus comme si vous vérifiez d'abord toutes les conditions préalables, en les éliminant, puis en progressant vers le code que vous souhaitez exécuter. En tant que tel, le code juteux que vous voulez vraiment exécuter appartient directement à la portée de la fonction; c'est vraiment la fonction.
Le deuxième cas se lit plus comme si les deux chemins sont un code valide à exécuter qui est tout aussi pertinent pour la fonction que l'autre en fonction d'une condition. En tant que tels, ils appartiennent à des niveaux de portée similaires les uns aux autres.

La seule situation que j'ai jamais vue vraiment importante est un code comme celui-ci:

List<?> toList() {
    if (left != null && right != null) {
        Arrays.asList(merge(left, right));
    }
    if (left != null) {
        return Arrays.asList(left);
    }
    if (right != null) {
        return Arrays.asList(right);
    }
    return Arrays.asList();
}

Il y a clairement quelque chose qui cloche ici. Le problème est qu'il n'est pas clair s'il returnfaut l'ajouter ou le Arrays.asListsupprimer. Vous ne pouvez pas résoudre ce problème sans une inspection plus approfondie des méthodes connexes. Vous pouvez éviter cette ambiguïté en utilisant toujours des blocs if-else exhaustifs. Cette:

List<?> toList() {
    if (left != null && right != null) {
        Arrays.asList(merge(left, right));
    } else if (left != null) {
        return Arrays.asList(left);
    } else if (right != null) {
        return Arrays.asList(right);
    } else {
        return Arrays.asList();
    }
}

ne compilera pas (dans les langues vérifiées statiquement) sauf si vous ajoutez d'abord un retour explicite if.

Certaines langues ne permettent même pas le premier style. J'essaie de ne l'utiliser que dans un contexte strictement impératif ou pour des conditions préalables dans les méthodes longues:

List<?> toList() {
    if (left == null || right == null) {
        throw new IllegalStateException()
    }
    // ...
    // long method
    // ...
}

Avoir trois sorties de la fonction comme celle-ci est vraiment déroutant si vous essayez de suivre le code et de mauvaises pratiques.

Si vous avez un seul point de sortie pour la fonction, les autres sont requis.

var result;
if(check1(value)
{
    result = -1;
}
else if(check2(value))
{
    result = value;
}
else 
{
    result = 0;
}
return result;

En fait, allons encore plus loin.

var result = 0;
var c1 = check1(value);
var c2 = check2(value);

if(c1)
{
    result = -1;
}
else if(c2)
{
    result = value;
}

return result;

Assez juste, vous pourriez plaider pour un retour anticipé unique sur la validation des entrées. Évitez simplement d'envelopper la masse entière si votre logique dans un if.

Je préfère omettre la déclaration else redondante. Chaque fois que je le vois (en révision ou refactorisation de code), je me demande si l'auteur a compris le flux de contrôle et qu'il pourrait y avoir un bug dans le code.

Si l'auteur pensait que la déclaration else était nécessaire et ne comprenait donc pas les implications du flux de contrôle de la déclaration de retour, un tel manque de compréhension est certainement une source majeure de bogues, dans cette fonction, et en général.