Utiliser ELSE est-il une mauvaise programmation? [fermé]

J'ai souvent rencontré des bogues causés par l'utilisation de la ELSEconstruction. Un excellent exemple est quelque chose du genre:

If (passwordCheck() == false){
    displayMessage();
}else{
    letThemIn();
}

Pour moi, cela crie un problème de sécurité. Je sais que passwordCheck est probablement un booléen, mais je n'y placerais pas la sécurité de mes applications. Que se passerait-il si c'est une chaîne, un entier, etc.?

J'essaie généralement d'éviter d'utiliser ELSE, et plutôt d'opter pour deux instructions IF complètement distinctes pour tester ce que j'attends. Tout le reste est alors ignoré OU est spécifiquement géré.

C'est sûrement une meilleure façon d'empêcher les bugs / problèmes de sécurité d'entrer dans votre application.

Comment faites-vous les gars?

Le elsebloc doit toujours comprendre ce que vous voulez que le comportement par défaut soit.

Il n'est pas nécessaire de les éviter, veillez simplement à les utiliser de manière appropriée.

Dans votre exemple, l'état par défaut doit être de ne pas autoriser l'accès. Un peu de refactoring vous laisse:

If (passwordCheck)
{
   letThemIn();
}
else
{
   displayMessage();
}

c'est-à-dire si la vérification du mot de passe fonctionne, laissez-les entrer, sinon il est toujours valide d'afficher un message d'erreur.

Vous pouvez bien sûr ajouter des vérifications supplémentaires à votre logique en utilisant else ifdes ifinstructions plutôt que des instructions complètement séparées .

Non, il n'y a rien de mal à cela ELSE. ELSEn'est pas le nouveau GOTO. En fait, l'utilisation de deux IFs au lieu de ELSEpeut entraîner plusieurs problèmes.

Exemple un:

if (this(is(a(very())==complex(check())))) {
   doSomething();
}

if (!this(is(a(very())==complex(check())))) {
   doTheOtherThing();
}

Vous voyez le copier-coller? Il attend juste le jour où vous changez l'un et oubliez l'autre.

Exemple deux:

foreach(x in y) {
  if (first_time) {
    first_time = false;
    doSomething();
  }

  if (!first_time) {
    doTheOtherThing();
  }
}

Comme vous pouvez le voir, le second IFsera également exécuté pour le premier élément, car la condition a déjà changé. Dans les programmes du monde réel, ces bogues sont plus difficiles à repérer.

Il y a toujours un AUTRE. Si vous écrivez

if(foo)
  bar();

tu écris

if(foo)
{
  bar();
}
else
{
   // do nothing
}

Tout ce que vous mettez dans le chemin ELSE est de votre responsabilité.

Personnellement, j'ai tendance à éviter elseautant que possible, mais ce n'est pas pour un problème de sécurité .

Lors de la lecture de code, les instructions imbriquées rendent plus difficile le respect de la logique, car vous devez vous rappeler quel ensemble de conditions y mènera. Pour cette raison, je suis un grand fan de sortie anticipée:

if (checkPassword != OK) { displayMessage(); return; }

letThemIn();

Cela s'applique également aux boucles foret whiledans lesquelles je vais utiliser continueet breakchaque fois que cela évite un niveau d'indentation.

Chris Lattner le dit mieux que moi dans les normes de codage LLVM .

Il suffit ensuite de les remplacer,

If (passwordCheck == true)
{
     letThemIn();
}
else
{
     displayMessage();
}

Comme Matthieu M., je préfère une sortie anticipée à d'autres blocs profondément imbriqués ... Cela illustre une programmation bien défensive (si mauvaises conditions, inutile de continuer). Beaucoup de gens seront en désaccord avec nous, préférant un point de sortie unique; ce n'est pas l'objet du débat (je pense).

Maintenant, j'utilise certainement elsequand cela a du sens, en particulier pour des alternatives simples et courtes. Comme dit, la duplication du test est une perte de temps (programmeur et CPU), source de confusion et, plus tard, de bugs (quand l'un est changé, pas l'autre).
Parfois, j'ajoute un commentaire sur la elsepièce, rappelant quelle était la condition (en particulier si la ifpièce est longue, par exemple dans le code hérité) ou quelle est l'alternative.

Notez que certains partisans extrêmes de la programmation fonctionnelle proposent de s'en débarrasser entièrement if, en faveur de la correspondance de motifs ... Un peu trop extrême à mon goût. :-)

Il n'y a rien de mal à utiliser ELSE. Cependant, cela peut conduire à un code trop complexe, difficile à lire et à comprendre. Cela peut indiquer une mauvaise conception. Cela indique certainement des cas d'utilisation supplémentaires qui devront être testés.

Essayez de supprimer les ELSE si vous le pouvez - mais ne soyez pas paranoïaque à ce sujet. Steve McConnell appelle ce code en ligne droite dans Code Complete. C'est-à-dire qu'il existe un chemin clair simple à travers votre code.

Approches pour essayer votre problème particulier:

• utiliser le polymorphisme. À la limite de votre système, validez les informations d'identification de sécurité de l'utilisateur. S'ils sont légitimes, renvoyez un objet de session - avec accès aux parties pertinentes du système ou lancez une exception. Cependant, cela pourrait rendre votre système plus complexe. Vous décidez donc de ce qui est plus facile à comprendre et à maintenir.

En général, les éléments suivants peuvent aider à réduire les ELSE dans votre code:

• de bonnes exigences peuvent réduire la nécessité de telles décisions dans le code. Vous n'avez peut-être pas du tout besoin d'implémenter le cas d'utilisation.
• conception plus claire. Cohésion maximale et minimisation du couplage. Cela garantit que les composants ne font pas double emploi avec les décisions prises dans d'autres composants.
• gestion des exceptions pour gérer les cas d'erreur.
• polymorphisme (voir l'exemple ci-dessus).
• instructions switch - ce sont des ELSE glorifiées mais elles sont meilleures dans certaines situations.

Votre supposition que le code soit une fuite de sécurité peut ou non être vraie selon la langue que vous utilisez. Dans le code C, cela pourrait être un problème (en particulier parce qu'en C, un booléen est juste un entier non nul ou nul) - mais dans la plupart des langages fortement typés (c'est-à-dire la vérification du type d'exécution) si la passwordCheckvariable a été déclarée comme booléenne, il n'y a aucun moyen de lui attribuer autre chose. En fait, tout dans un ifprédicat doit se résoudre en booléen, que vous utilisiez les opérateurs booléens ou que vous utilisiez simplement la valeur. Si vous parvenez à avoir un autre type d'objet lié au passwordCheckruntime, cela lèverait un certain type d'exception de conversion illégale.

Les constructions if / else simples sont beaucoup plus faciles à lire que les constructions if / if - et moins sujettes à des problèmes par inadvertance si quelqu'un essaie de retourner la construction. Prenons le même exemple pendant une seconde:

if(passwordCheck == false) {
    denyAccess();
}

if(passwordCheck) {
    letThemIn();
}

La signification des clauses mutuellement exclusives que vous souhaitez exécuter ci-dessus est perdue. C'est ce que la construction if / else transmet. Deux branches d'exécution mutuellement exclusives, où l'une d'elles s'exécutera toujours. Il s'agit d'un élément important de la sécurité: il n'y a aucun moyen de le faire letThemInaprès avoir appelé denyAccess.

Pour des raisons de clarté du code et pour garantir que les sections critiques sont les mieux protégées, elles doivent se trouver à l'intérieur de la clause primaire (la ifpartie). Le comportement non conforme par défaut doit se trouver dans la clause alternative (la elsepartie). Par exemple:

if(passwordCheck) {
    letThemIn();
} else {
    denyAccess();
}

REMARQUE: en travaillant avec différentes langues, j'ai développé un habbit de codage qui permet d'éviter la question "et si c'est une chaîne?" Il s'agit essentiellement de mettre la constante en premier dans l'expression booléenne. Par exemple, au lieu de vérifier, passwordCheck == falseje vérifie false == passwordCheck. Cela évite également le problème d'affectation accidentelle possible en C ++. En utilisant cette approche, le compilateur se plaindra si je tape à la =place de ==. Dans des langages comme Java et C #, le compilateur traiterait l'affectation dans la clause if comme une erreur, mais C ++ l'acceptera volontiers. C'est pourquoi j'ai également tendance à faire une vérification nulle avec le nullpremier.

Si vous changez régulièrement de langue, placer la constante en premier est très utile. Cependant, dans mon équipe, il est opposé à la norme de codage et le compilateur détecte de toute façon ces problèmes. Cela peut être difficile à briser.

Dire que l'utilisation elselorsque la programmation est mauvaise, c'est comme dire que l'utilisation otherwiselorsque l'on parle est mauvaise.

Bien sûr, ils peuvent tous les deux être utilisés de mauvaise façon, mais cela ne signifie pas qu'ils doivent être évités simplement parce que vous avez fait une erreur qui les a inclus. Je ne serais pas surpris si de nombreux bugs dépendaient d'un defaultcas manquant dans une switchdéclaration.

Considérez Elsecomme une liste blanche de votre flux d'application. Vous vérifiez les conditions qui DEVRAIENT permettre au flux d'application de continuer, et si elles ne sont pas remplies, votre Elseest exécuté pour résoudre le problème, interrompre l'exécution de l'application ou quelque chose de similaire.

Else en soi n'est pas mauvais, mais si vous l'utilisez mal, vous pouvez voir des effets indésirables.

En ce qui concerne également votre déclaration

"Je sais que passwordCheck est susceptible d'être un booléen, mais je n'y placerais pas la sécurité de mes applications."

Pour les méthodes que vous développez, TOUJOURS renvoyer un type de données. Bien que PHP Core soit jonché de code qui retourne deux types de données ou plus, c'est une mauvaise pratique car cela permet de deviner les appels de fonction. Si vous devez renvoyer plus d'un type de données, envisagez de lever une exception (je trouve que c'est souvent la raison pour laquelle je voudrais retourner un autre type de données - quelque chose s'est horriblement mal passé), ou envisagez de restructurer votre code afin que vous puissiez ne renvoie qu'un seul type de données.

Tout d'abord. LOL! Il n'y a aucune raison d'éviter quoi que ce soit d'autre. Ce n'est PAS une mauvaise pratique en aucune façon, forme ou forme.

Si quelque chose le code doit être

if(!IsLoggedIn) { ShowBadLoginOrNoAccessPage(); return }

Il n'y a pas deux ifs et il n'y en a pas d'autre. C'est ce que je fais dans toutes mes applications sauf une dans laquelle je lève une exception. L'exception est interceptée dans ma fonction qui vérifie l'URL pour la page appropriée à afficher (ou bien je peux mettre la fonction d'erreur catch / check dans asp.net). Il imprime une page générique qui dit de ne pas autoriser ou quel que soit le message que j'utilise dans l'exception (je vérifie toujours le type d'exception et définit le code d'état http).

-Edit- comme le montre l'exemple ammoQ deux ifs est ridicule. Vraiment d' autre est tout aussi bon ou meilleur qu'un if. Si quelque chose doit être évité (même si je ne le fais pas personnellement. Mais j'utilise beaucoup le retour et la rupture), comme il a été dit, plus de chemins de code augmentent la probabilité de bugs. Voir la complexité cyclomatique

-Modifier 2- Si vous vous inquiétez de l'utilisation if / else. Je noterai également que ma préférence est de mettre le bloc de code le plus court en haut, comme

if(cond == false) {
    a()
    b()
    c()
    onetwothree()
}
else
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}

Plutôt que

if(cond) 
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}
else
{
    a()
    b()
    c()
    onetwothree()
}

J'aime définir une valeur par défaut avant les conditions lorsque je le peux. J'ai l'impression que c'est un peu plus facile à lire et un peu plus explicite, mais ce n'est qu'une préférence. J'ai tendance à essayer d'éviter les conditions négatives dans mon code. Je ne suis pas un grand fan de la vérification de! Foo ou false == foo et j'ai l'impression que c'est une sorte d'équivalent conditionnel d'un négatif.

foo = bar;

if ('fubar' == baz) {
    foo = baz;
}

au lieu de ...

if ('fubar' == baz) {
    foo = baz;
} else {
    foo = bar;
}

Le bloc de code précédent semble juste un peu plus facile à lire pour moi. Il me semble plus naturel d'avoir une sorte de paranoïa sceptique sur mon code. Définir une valeur par défaut quelle que soit la condition me met à l'aise: P

Je dirais que l'utilisation de la logique de branchement de toute nature doit être évitée autant que possible. Bien que rien ne cloche avec ELSE ou IF, il existe de nombreuses façons d'écrire du code pour minimiser la nécessité d'utiliser une logique de branchement. Je ne dis pas que la logique de branchement peut être totalement éliminée - elle sera nécessaire à certains endroits - mais il est possible de refactoriser le code pour en éliminer une bonne partie. Dans la plupart des cas, cela améliorera l'intelligibilité et la précision de votre code.

À titre d'exemple, les opérateurs ternaires sont également généralement de bons candidats:

If VIP Then 
  Discount = .25
Else
  Discount = 0
End If
Total = (1 - Discount) * Total

En utilisant une approche ternaire:

Discount = VIP ? .25 : 0
Total = (1 - Discount) * Total

Les opérateurs ternaires déplacent la branche vers la droite dans le bon sens.