Les affectations dans la partie conditionnelle des conditionnelles sont-elles une mauvaise pratique?

35

Supposons que je veuille écrire une fonction qui concatène deux chaînes en C. Voici comment je l'écrirais:

void concat(char s[], char t[]){
    int i = 0;
    int j = 0;

    while (s[i] != '\0'){
        i++;
    }

    while (t[j] != '\0'){
        s[i] = t[j];
        i++;
        j++;
    }

    s[i] = '\0';

}

Cependant, K & R dans leur livre l'a implémenté différemment, en incluant notamment autant que possible dans la partie condition de la boucle while:

void concat(char s[], char t[]){
    int i, j;
    i = j = 0;
    while (s[i] != '\0') i++;

    while ((s[i++]=t[j++]) != '\0');

}

Quel chemin est préféré? Est-il encouragé ou découragé d’écrire du code comme le fait K & R? Je crois que ma version serait plus facile à lire par d'autres personnes.

Richard Smith
la source
38
N'oubliez pas que K & R a été publié pour la première fois en 1978. Il y a eu quelques petits changements dans la manière dont nous codons depuis.
CorsiKa
28
La lisibilité était très différente à l'époque des téléimprimeurs et des éditeurs orientés ligne. Le regroupement de tous ces éléments sur une seule ligne était généralement plus lisible.
user2357112 prend en charge Monica
15
Je suis choqué qu'ils aient des indices et des comparaisons avec '\ 0' au lieu de quelque chose comme while (*s++ = *t++); (Mon C est très rouillé, ai-je besoin de parens pour la préséance des opérateurs?) K & R a-t-il publié une nouvelle version de leur livre? Leur livre original avait un code extrêmement concis et idiomatique.
utilisateur949300
4
La lisibilité est une chose très personnelle, même à l’époque des télétypes. Différentes personnes préfèrent des styles différents. Une grande partie des instructions fastidieuses concernait la génération de code. À cette époque, certains jeux d'instructions (par exemple, Données générales) pouvaient regrouper plusieurs opérations en une seule instruction. En outre, au début des années 80, il existait un mythe selon lequel l'utilisation de parenthèses générait davantage d'instructions. J'ai dû générer l'assembleur pour prouver au réviseur de code que c'était un mythe.
tasse
10
Notez que les deux blocs de code ne sont pas équivalents. Le premier bloc de code ne copie pas la terminaison '\0'de t(les whilesorties en premier). Cela laissera la schaîne résultante sans terminaison '\0'(à moins que l'emplacement mémoire ait déjà été mis à zéro). Le deuxième bloc de code effectuera la copie de la terminaison '\0'avant de quitter la whileboucle.
Makyen

Réponses:

80

Préférez toujours la clarté à l'intelligence. Auparavant, le meilleur programmeur était celui dont personne ne pouvait comprendre le code. "Je ne peux pas donner de sens à son code, il doit être un génie" , ont-ils déclaré. De nos jours, le meilleur programmeur est celui dont tout le monde peut comprendre le code. Le temps passé sur l’ordinateur est moins cher que le temps du programmeur.

N'importe quel imbécile peut écrire du code qu'un ordinateur peut comprendre. Les bons programmeurs écrivent du code que les humains peuvent comprendre. (M. Fowler)

Donc, sans aucun doute, je choisirais l'option A. Et c'est ma réponse définitive.

Tulains Córdova
la source
8
Une idéologie très pithy, mais le fait demeure qu'il n'y a rien de mal à une cession conditionnelle. Il est de loin préférable de sortir tôt d'une boucle ou de dupliquer du code avant et à l'intérieur d'une boucle.
Miles Rout
26
@MilesRout Il y a. Il y a quelque chose qui cloche dans le fait qu'un code ayant un effet secondaire ne vous attendez pas, par exemple, passer des arguments de fonction ou évaluer des conditions. Sans parler de ce qui if (a=b)peut facilement être confondu avec if (a==b).
Arthur Havlicek
12
@Luke: "Mon IDE peut nettoyer X, donc ce n'est pas un problème" n'est pas convaincant. Si ce n'est pas un problème, pourquoi l'IDE facilite-t-il la tâche de "réparation"?
Kevin
6
@ArthurHavlicek Je suis d'accord avec votre argument général, mais le code avec effets secondaires conditionnels n'est vraiment pas si rare: c'est while ((c = fgetc(file)) != EOF)le premier qui me vienne à l'esprit.
Daniel Jour
3
+1 "Considérant que le débogage est deux fois plus difficile que d'écrire un programme en premier lieu, si vous êtes aussi intelligent que vous le pouvez lorsque vous l'écrivez, comment allez-vous le déboguer?" BWKernighan
Christophe
32

La règle d'or, comme dans la réponse de Tulains Córdova, est de s'assurer d'écrire du code intelligible. Mais je ne suis pas d'accord avec la conclusion. Cette règle d’or veut dire écrire du code que le programmeur typique qui finira par maintenir votre code peut comprendre. Et vous êtes le meilleur juge pour savoir qui est le programmeur typique qui conservera votre code.

Pour les programmeurs qui n'ont pas commencé par le C, la première version est probablement plus facile à comprendre, pour des raisons que vous connaissez déjà.

Pour ceux qui ont grandi avec ce style de C, la deuxième version est peut-être plus facile à comprendre: pour eux, on comprend également ce que le code fait, pour eux, cela laisse moins de questions sur la raison pour laquelle il est écrit ainsi, et pour eux. , moins d’espace vertical signifie que plus de contexte peut être affiché à l’écran.

Vous devrez compter sur votre propre bon sens. À quel public voulez-vous que votre code soit le plus facile à comprendre? Ce code est-il écrit pour une entreprise? Ensuite, le public cible est probablement les autres programmeurs de cette société. Est-ce un projet de loisir personnel sur lequel personne ne travaillera que vous-même? Ensuite, vous êtes votre propre public cible. Est-ce que vous voulez partager ce code avec d'autres? Ensuite, ces autres sont votre public cible. Choisissez la version qui correspond à ce public. Malheureusement, il n'y a pas de moyen privilégié d'encourager.

hvd
la source
14

EDIT: La ligne a s[i] = '\0';été ajoutée à la première version, elle a donc été corrigée comme décrit dans la variante 1 ci-dessous, de sorte que cela ne s'applique plus à la version actuelle du code de la question.

La seconde version a l'avantage d'être correcte , alors que la première ne l'est pas - elle ne termine pas correctement la chaîne de caractères cible.

L '"affectation en condition" permet d'exprimer le concept de "copier chaque caractère avant de rechercher le caractère nul" de manière très concise et de manière à faciliter l'optimisation du compilateur, bien que de nombreux ingénieurs en logiciels trouvent ce style de code moins lisible. . Si vous insistez pour utiliser la première version, vous devrez soit

  1. ajoutez la terminaison nulle après la fin de la deuxième boucle (ajouter plus de code, mais vous pouvez dire que la facilité de lecture le rend rentable) ou
  2. changez le corps de la boucle en "affectez d’abord, puis vérifiez ou enregistrez le caractère attribué, puis incrémentez les indices". Vérifier l'état au milieu de la boucle signifie sortir de la boucle (réduction de la clarté, mal vu par la plupart des puristes). Sauvegarder le caractère attribué signifierait introduire une variable temporaire (réduction de la clarté et de l'efficacité). Les deux de ceux-ci annihileraient l'avantage à mon avis.
hjhill
la source
Correct est mieux que lisible et concis.
utilisateur949300
5

Les réponses de Tulains Córdova et hvd couvrent assez bien les aspects clarté / lisibilité. Permettez-moi d’ envisager une autre raison en faveur des affectations dans certaines conditions. Une variable déclarée dans la condition est uniquement disponible dans la portée de cette instruction. Vous ne pouvez pas utiliser cette variable par la suite par accident. La boucle for fait cela depuis des lustres. Et il est assez important que le prochain C ++ 17 introduit une syntaxe similaire pour if et switch :

if (int foo = bar(); foo > 42) {
    do_stuff();
}

foo = 23;   // compiler error: foo is not in scope
besc
la source
3

Non, c'est un style C très standard et normal. Votre exemple est mauvais, car il ne devrait s'agir que d'une boucle perdue, mais en général, il n'y a rien de mal à

if ((a = f()) != NULL)
    ...

par exemple (ou avec while).

Miles Rout
la source
7
Il y a quelque chose qui cloche avec ça; `! = NULL` et ses parents dans un C conditionnel sont natifs, juste pour apaiser les développeurs qui ne sont pas à l'aise avec le concept de valeur vraie ou fausse (ou vice-versa).
Jonathan Cast
1
@jcast Non, il est plus explicite d'inclure != NULL.
Miles Rout
1
Non, c'est plus explicite à dire (x != NULL) != 0. Après tout, c’est ce que C vérifie vraiment , non?
Jonathan Cast
@jcast Non, ça ne l'est pas. Vérifier si quelque chose n'est pas égal à faux n'est pas comment vous écrivez des conditionnels dans n'importe quelle langue.
Miles Rout
"Vérifier si quelque chose n'est pas égal à faux n'est pas comment vous écrivez des conditionnels dans n'importe quelle langue." Exactement.
Jonathan Cast
2

Au temps de K & R

  • 'C' était un code d'assemblage portable
  • Il était utilisé par les programmeurs qui pensaient en code assembleur
  • Le compilateur n'a pas fait beaucoup d'optimisation
  • La plupart des ordinateurs avaient des «jeux d’instructions complexes». Par exemple, while ((s[i++]=t[j++]) != '\0')ils correspondraient à une instruction sur la plupart des processeurs (je suppose le VAC de décembre).

Il y a des jours

  • La plupart des lecteurs de code C ne sont pas des programmeurs de code assembleur
  • Les compilateurs C effectuent de nombreuses optimisations. Par conséquent, le code plus simple à lire est susceptible d'être traduit dans le même code machine.

(Remarque sur l'utilisation systématique d'accolades - le premier ensemble de code occupe plus de place en raison de «inutiles» {}, ce qui, selon mon expérience, empêche souvent le code mal fusionné à partir du compilateur et autorise les erreurs avec des emplacements «;» incorrects. détecté par des outils.)

Cependant, dans les temps anciens, la 2ème version du code aurait été lue. (Si j'ai bien compris!)

concat(char* s, char *t){      
    while (*s++);
    --s;
    while (*s++=*t++);
}
Ian
la source
2

Même être capable de faire cela est une très mauvaise idée. Il est familièrement appelé "Le dernier bogue du monde", ainsi:

if (alert = CODE_RED)
{
   launch_nukes();
}

Alors que vous n'êtes pas susceptible de faire une erreur qui est tout à fait si grave, il est très facile de visser accidentellement et provoquer une erreur difficile à trouver dans votre base de code. La plupart des compilateurs modernes insèrent un avertissement pour les assignations dans un conditionnel. Ils sont là pour une raison, et vous feriez bien de les écouter et d'éviter simplement cette construction.

Maçon Wheeler
la source
Avant ces avertissements, nous écririons CODE_RED = alertdonc une erreur de compilation a été générée.
Ian
4
@Ian Yoda Conditionals qui s'appelle. Difficile à lire, ils sont. Malheureusement, leur nécessité est.
Mason Wheeler
Après une très brève période d’initiation, les conditions de Yoda ne sont pas plus difficiles à lire que les conditions normales. Parfois, ils sont plus lisibles . Par exemple, si vous avez une séquence de ifs / elseifs, le fait que la condition soit testée à gauche pour une emphase plus élevée constitue une légère amélioration, IMO.
user949300
2
@ user949300 Deux mots: Syndrome de Stockholm: P
Mason Wheeler
2

Les deux styles sont bien formés, corrects et appropriés. Le choix le plus approprié dépend en grande partie des directives de style de votre entreprise. Les IDE modernes faciliteront l’utilisation des deux styles grâce à l’utilisation de filtres de syntaxe dynamiques qui mettront en évidence les zones qui auraient autrement pu devenir source de confusion.

Par exemple, l'expression suivante est mise en surbrillance par Netbeans :

if($a = someFunction())

pour "assignation accidentelle".

entrez la description de l'image ici

Pour dire explicitement à Netbeans que "oui, je voulais vraiment faire ça ...", l'expression peut être entourée d'un ensemble de parenthèses.

if(($a = someFunction()))

entrez la description de l'image ici

En fin de compte, tout se résume aux directives de style de l'entreprise et à la disponibilité d'outils modernes facilitant le processus de développement.

Luke A. Leber
la source