En ce qui concerne la maintenance, est-ce que `` else while '' sans accolades intermédiaires est considéré comme sûr?

26

Les else whileentretoises sans intervention sont-elles considérées comme «sûres» pour la maintenance?

Écrire du if-elsecode sans accolades comme ci-dessous ...

if (blah)
    foo();
else
    bar();

... comporte un risque car le manque d'accolades permet de changer très facilement la signification du code par inadvertance.

Cependant, ci-dessous est-il également risqué?

if (blah)
{
    ...
}
else while (!bloop())
{
    bar();
}

Ou else whilesans accolades est-il considéré comme «sûr»?

Mehrdad
la source
20
pour moi, else whilec'est dégueu. J'utiliserais else { while (condition) { ... } }.
Joachim Sauer
7
Je pense que c'est trop subjectif pour une réponse correcte ... Je ne le ferais pas moi-même car je ne m'attends pas à une boucle là-bas, et je pense que je ne m'attends pas à rendre la lisibilité difficile.
johannes
7
Je voudrais extraire la méthode pour tout-stuff
moucher
12
Honnêtement, ça me donne la chair de poule. ifn'est évalué qu'une seule fois, mais whiledénote une boucle, donc la connexion des deux me donne un sentiment non étayé que cela iffait partie de la boucle ... en quelque sorte ...
user281377
4
Et si, dans la elseclause, vous voulez faire while et faire quelque chose de plus? Utilisez simplement des accolades, s'il vous plaît.
Carlos Campderrós

Réponses:

57

Cela me rappelle ce code:

if ( ... ) try {
..
} catch (Exception e) {
..
} else {
...
}

Chaque fois que vous combinez deux types de blocs, oubliant les accolades et n'augmentant pas l'indentation, vous créez du code très difficile à comprendre et à maintenir.

Sulthan
la source
18
Bon exemple. Quelle horrible façon de l'écrire.
Leo
4
Wow, cette réponse est ridiculement convaincante!
Mehrdad
Vous pouvez facilement configurer des IDE modernes pour formater automatiquement le code lors de l'enregistrement - y compris l'insertion d'accolades et la correction de l'indentation. Ce n'est donc qu'un demi-argument. Correctement indenté, cela ne poserait aucun problème de lisibilité, qu'il y ait des accolades ou non.
Hans-Peter Störr
55

C'est peut-être parce que j'ai appris mon métier (il y a longtemps ) en utilisant la méthode Jackson Entity Structure Diagram , mais je souscris à l'idée que le seul terme correct sans accolade après un ifou un elseest un suivant if(c'est-à-dire autoriser une else iféchelle)

Tout autre élément (sans jeu de mots) laisse la possibilité de malentendus et / ou de problèmes de maintenance. C'est l'aspect central de l'idée que les PO sont «dangereux».

Je serais également très hésitant à inclure le while()sur la même ligne que le else- qu'il soit contreventé ou non. Cela ne me semble pas juste ... l'absence de masques d'indentation supplémentaires que c'est la elseclause. Et le manque de clarté crée des malentendus (voir ci-dessus).

Donc, dans l'exemple, je conseillerais / recommanderais fortement (et insisterais, dans mon équipe) sur:

if ( blah )
{
    ...
}
else
{
    while ( !bloop() )
    {
        bar();
    }
}

Bien sûr, je m'attendrais également à voir des commentaires appropriés aussi.

-- Modifier --

Récemment, Apple a souffert d'une vulnérabilité SSL, causée par un correctif de maintenance médiocre - qui a ajouté une deuxième ligne à une seule ligne sans contreventement. Mettons donc de côté l'idée que les lignes simples sans accolade sont OK?

Andrew
la source
2
Je suis d'accord si c'est le seul terme correct pour suivre un autre. S'il y avait un else whileje ne remarquerais probablement même pas qu'il y avait une boucle dans un survol rapide du code, surtout si la condition que je recherchais était satisfaite par le si.
Drake Clarris
3
C'est marrant. Tout le monde dit qu'il est plus facile de lire si tout est entre accolades. Je trouve le contraire vrai. S'il ne s'agit que d'une seule instruction, le garder hors des accolades facilite la lecture. Moins d'encombrement. C'est peut-être uniquement parce que je l'ai toujours fait de cette façon.
Jeff Davis
2
@JeffDavis c'est une bonne prise. "Plus facile à lire" est une invitation typique à une guerre sainte inutile. Pour ma part, je préfère les accolades, mais pas à cause des ordures «plus faciles à lire» non pertinentes (pour moi, par exemple, c'est exactement le contraire), mais parce qu'il est plus difficile d'interrompre la maintenance du code. Par l'OP façon orthographié mieux dans leur question: est else whilesans accolades intervenir considérés comme « sûrs » ?
moucher
@JeffDavis - Je sais que ce fil a deux ans, mais Apple a récemment découvert pourquoi ne pas utiliser d'accolades n'est pas une bonne idée andrewbanks.com/…
Andrew
@Andrew Au fait, le langage Swift d'Apple interdit désormais explicitement le contrôle de flux sur une ligne. Ce bug pourrait être l'une des raisons de le faire.
Sulthan
6

On m'a toujours appris à tout garder entre accolades, en retrait et commenté. Je pense que c'est beaucoup plus facile à lire et à repérer les erreurs. Personnellement, je pense que la modularité est la clé, donc j'écrirais toujours le code comme ceci:

    if(blah)
    {
     ....
    }
    else
    {
       while(!bloop()
       {
        bar;
       }
    }
CodeCompileHack
la source
6

Je voudrais extraire la méthode et la faire

if ( blah )
{
    ...
}
else
{
   newMethod();
}

Voir l'approche "extraire la méthode" expliquée sur le site du catalogue de refactorisation :

Vous disposez d'un fragment de code qui peut être regroupé.

Transformez le fragment en une méthode dont le nom explique le but de la méthode.

void printOwing() {
    printBanner();

    //print details
    System.out.println ("name:    " + _name);
    System.out.println ("amount    " + getOutstanding());
}

                                                                                                         http://www.refactoring.com/catalog/arrow.gif

void printOwing() {
    printBanner();
    printDetails(getOutstanding());
}

void printDetails (double outstanding) {
    System.out.println ("name:    " + _name);
    System.out.println ("amount    " + outstanding);
}
user470365
la source
Cela dépend ... si, par exemple, la boucle while utilise des données de niveau fonction, vous devez maintenant augmenter ces données au niveau du module (ou de la classe, si vous utilisez C ++). Et si ce n'est qu'un petit extrait de code, vous êtes dans des méthodes triviales. Mais parfois, selon les circonstances, je suis d'accord.
Andrew
1
+1 Les compilateurs Smart C ++ peuvent incorporer une fonction. Dans un environnement .Net, les petites fonctions sont meilleures grâce à JIT.
Job
4

Je le considérerais comme une mauvaise habitude de codage. Parfois, cela fonctionne parfaitement bien et vous n'aurez aucun problème pour compiler et exécuter le code. À d'autres moments, cela peut provoquer de graves bogues et vous finirez par passer des heures à le corriger.

Il est toujours recommandé de modulariser votre code. Si vous devez mettre une boucle while dans une autre partie, placez-la dans un bloc afin que les autres, lorsqu'ils travaillent sur votre code, puissent facilement comprendre la logique.

if ( blah )
{
    ...
}
else
{
    while ( !bloop() )
    {
        bar();
    }
}

C'est une bonne pratique de mettre du code en blocs. Cela le rend plus simple et plus facile à comprendre et à déboguer également.

Sandeep Gupta
la source
4

Cela pourrait être bien si cela reste simple comme ça, bien que personnellement je ne l'aime pas et ma préférence est d'utiliser des accolades même pour les blocs de code if / else les plus simples. C'est juste plus propre pour moi.

Cependant, lorsque cela devient compliqué, c'est lorsque vous avez imbriqué if / else en boucle avec des accolades, d'autres sans. Une boucle While au milieu de tous ces spaghettis! J'ai si mal travaillé avec du code et c'est un cauchemar à déboguer et à comprendre. Cela découle du premier point, c'est bien quand cela reste simple et que vous en êtes satisfait, mais ensuite d'autres programmeurs viennent et y ajoutent des choses, probablement un si autre dans la boucle while. Si c'est écrit en premier lieu, il y a moins de chances que cela se produise.

Le but est de clarifier les choses afin que les autres programmeurs puissent voir en un coup d'œil ce qui est bien ou mal. Écrivez le code de sorte que le motif soit correct et ne fasse pas de bruit.

Le revers de la médaille est que je pourrais peut-être voir certaines personnes affirmer que, dans certains cas, cela se lit bien. Si j'ai la ressource dont j'ai besoin pour faire ce traitement autrement pendant que j'attends quelque chose, faites-le. Même pour moi, il n'y a toujours pas de différence dans l'imbrication de la boucle while à l'intérieur du bloc else.

Daniel Hollinrake
la source
3

Eh bien, malgré que tout le monde se disputent sur l'utilisation d'appareils orthodontiques et semblent les aimer, je préfère en fait le contraire. Je n'utilise des accolades que lorsque je le dois car je le trouve plus lisible et concis sans.

if (...)
   foo();
else while(...)
   bar();

... Je trouve ce " else while(...)" remarquablement lisible! Il se lit même comme un anglais ordinaire! Mais je suppose que les gens trouveront tous ça étrange parce que c'est pour le moins inhabituel.

En fin de compte, nous avons tous tendance à le rendre à l'épreuve des idiots avec des accolades ... parce que, eh bien, vous savez.

dagnelies
la source
3
ce sera particulièrement lisible après que des mainteneurs innocents auront changé else while(...) bar();quelque chose comme else while(...) foobar(); bar();:)
gnat
3
eh bien, je dirais que c'est un mainteneur innocent assez stupide et dangereux . Je connais le principe de l'abaisser pour éviter les erreurs stupides ... mais aller si loin est quand même assez triste. Mais je savais que j'obtiendrais de tels commentaires et votes négatifs. Aucun problème.
dagnelies
2
"Toujours coder comme si la personne qui maintiendra votre code est un psychopathe violent qui sait où vous vivez." ( Alan Braggins )
moucheron
1
Eh bien, tout se résume à ce que vous supposez être le niveau le plus stupide de quelqu'un qui travaille sur votre code. Je soutiens que si votre "responsable" devient confus par ces quatre lignes de code ... alors vous avez un gros problème à résoudre. Et si c'est un psycho violent, vous en avez deux. ;)
dagnelies
1
si - eh bien le lien que je mentionne ci-dessus a une version développée de cette citation: Programmeur 1: 'Il y a une belle citation ici - "Toujours coder comme si la personne qui maintiendra votre code est un psychopathe violent qui sait où vous vivez"' . Programmeur 2: (regarde le responsable) 'Que voulez-vous dire "comme si"?'
moucher
2

Je mets toujours des accolades, juste parce que vous pourriez ajouter une autre ligne lorsque vous êtes pressé, la mettre en retrait, oublier les accolades et vous gratter la tête de ce qui se passe. Je garde le support d'ouverture sur la même ligne, mais cela n'a pas d'importance. Dans les deux cas, il vaut mieux les avoir.

if(blah) {
    foo();
}
else {
    bar();
}
Tsvetomir Dimitrov
la source
Le positionnement des accolades déclenche probablement plus d'arguments que tout ce qui dépasse (ou peut-être y compris) la religion!
Andrew
D'accord. Éditera ma réponse pour souligner leur existence, pas leur position.
Tsvetomir Dimitrov
2

Qu'est-ce qui ne va pas avec les accolades que tant de gens essaient d'éviter de les écrire?

Quel problème résout exactement else while?

Les accolades sont bon marché et bonnes, et elles rendent l'intention du code évidente et simple, par opposition à intelligent et plein d'esprit.

Citant la philosophie Unix:

Règle de clarté: la clarté est meilleure que l'intelligence.

Parce que la maintenance est si importante et si chère, écrivez des programmes comme si la communication la plus importante qu'ils faisaient n'était pas vers l'ordinateur qui les exécutait mais vers les êtres humains qui liront et maintiendront le code source à l'avenir (vous y compris).

Tulains Córdova
la source
1

Il n'y a rien de mal à

if (blah)
    foo();
else
    bar();

tout comme il n'y a rien de mal à

if (blah) foo(); else bar();

dans les bonnes circonstances (vos circonstances peuvent varier, mais ce style particulier est mieux utilisé lorsque vous avez beaucoup de code répétitif - alors la vue de chaque ligne est plus importante que l'indentation stylistique)

Mais si vous choisissez une façon, respectez-la - la cohérence est la règle. Donc, votre deuxième exemple est très mauvais, car l'expression if avait des crochets pour son instruction, l'instruction while devrait être à l'intérieur des crochets de la clause else, pas à l'extérieur.


d'ailleurs j'ai déjà vu ce code:

if (x) foo()
{
  bar();
}

et il a été écrit par le standard de codage nazi lui-même (qui a insisté sur les crochets pour tout).

gbjbaanb
la source
1

Les IDE modernes peuvent facilement être configurés pour reformater (y compris supprimer ou ajouter des accolades inutiles) et / ou réindenter le code lors de l'enregistrement d'un fichier. Ainsi, votre exemple ressemblerait automatiquement par exemple

if (blah) {
  blub();
} else
  while (!bloop()) {
    bar();
  }

L'indentation rend toujours la nidification facilement visible, même sans accolades inutiles. Donc, si vous parvenez à appliquer ces paramètres IDE, je ne vois aucun risque.

Personnellement, je trouve le code qui omet les accolades et les sauts de ligne beaucoup plus lisibles car il évite l'encombrement:

if (blah) blub();
else while (!bloop()) bar();

Mais bien sûr, de nombreux développeurs sont très heureux de discuter de telles choses pour toujours et un jour.

Hans-Peter Störr
la source