Affectation booléenne des meilleures pratiques [clôturé]

10

J'ai rencontré le conditionnel suivant dans un programme que j'ai repris d'un autre développeur:

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.NeedsChange = false;
}

Je crois que ce code est redondant et laid, donc je l'ai changé pour ce que je pensais être une simple affectation booléenne basée sur une comparaison:

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE;

En voyant cela, quelqu'un examinant mon code a déclaré que bien que mon changement soit fonctionnellement correct, cela pourrait dérouter quelqu'un d'autre qui le regarde. Il pense que l'utilisation d'un opérateur ternaire rend cette affectation plus claire, alors que je n'aime pas l'ajout de code plus redondant:

obj.NeedsChange = (obj.Performance <= LOW_PERFORMANCE) ? true : false;

Son raisonnement est que faire quelque chose de la manière la plus concise ne vaut pas la peine, si cela oblige un autre développeur à s'arrêter et à comprendre exactement ce que vous avez fait.

La vraie question ici est laquelle de ces trois méthodes d'attribution d'une valeur au booléen obj.NeedsChangeest la plus claire et la plus maintenable?

Zach Posten
la source
25
La troisième forme est ridicule; c'est simplement énoncer ce qui devrait déjà être manifestement évident dans la deuxième forme.
Robert Harvey
6
Cela dépend entièrement de vos préférences personnelles. Nous pouvons prétendre le contraire, mais parce qu'ils sont tous fonctionnellement équivalents, cela se résume au style . Bien sûr, il y a une différence de lisibilité, mais mon "lisible et transparent" pourrait être votre "obtus et opaque"
MetaFight
3
@scriptin 5-8 lignes v 1 ligne est plus que la préférence, la ligne 5-8 est généralement plus claire et meilleure pour elle. Dans cet exemple simple, je préfère la ligne 1, mais en général, j'ai vu trop de 10 doublures obscurcies en 1 doublure pour plus de confort. Étant donné que je ne me plaindrais jamais de la variante 1, elle n'est peut-être pas jolie mais elle fait le travail tout aussi clairement et évidemment.
gbjbaanb
4
Les options 1 et 3 me disent "L'auteur ne comprend pas vraiment la logique booléenne".
17 de 26
2
La variante 1 peut être utile si vous devez souvent définir un point d'arrêt qui dépend de la valeur.
Ian

Réponses:

39

Je préfère 2, mais je pourrais opter pour un petit ajustement:

obj.NeedsChange = ( obj.Performance <= LOW_PERFORMANCE );

Pour moi, les parenthèses facilitent l'analyse de la ligne et indiquent clairement en un coup d'œil que vous attribuez le résultat d'une comparaison et n'effectuez pas de double affectation. Je ne sais pas pourquoi c'est (car je ne peux pas penser à une langue où les parenthèses empêcheraient en fait une double affectation), mais si vous devez satisfaire votre critique, alors ce sera peut-être un compromis acceptable.

Jacob Raihle
la source
4
c'est la bonne réponse - bien que le code dans la question soit correct, l'ajout de parenthèses indique au lecteur qu'il ne s'agit pas d'une affectation. Si vous parcouriez rapidement le code, les crochets vous donnent ces informations supplémentaires instantanées qui vous empêchent de regarder de plus près pour voir si le code voulait être comme ça, et ce n'était pas un bug accidentel. Par exemple, imaginez que la ligne était a = b == c, vouliez-vous assigner un booléen ou vouliez-vous assigner c à la fois à b et à a.
gbjbaanb
Les parenthèses empêcheraient une double affectation en Python. Même dans les langues où elles n'empêchent pas la double affectation, les parenthèses aident certainement à indiquer que vous avez affaire à deux types d'opérations.
user2357112 prend en charge Monica
23

La variante 1 est facile à comprendre, mais c'est son seul avantage. Je suppose automatiquement que quiconque écrit comme ça ne comprend pas vraiment ce que sont les booléens, et écrira un code infantile de la même manière à bien d'autres égards.

La variante 2 est ce que j'écrivais toujours et je m'attendais à lire. Je pense que quiconque est confus par cet idiome ne devrait pas être un écrivain professionnel de logiciels.

La variante 3 combine les inconvénients à la fois de 1 et de 2 '.

Kilian Foth
la source
Eh bien, la variante 1 partage son avantage avec la variante 2 ...
Deduplicator
1
+1 pour le code infantile. Je regarde ce code depuis des années, il me manquait juste le bon mot pour l'identifier.
Lilienthal
1
Ma première hypothèse avec un code comme la variante 1 est que les deux branches à un moment donné dans le passé étaient plus compliquées et que personne ne faisait attention lors de la refactorisation. Si toutefois c'était comme ça quand il a été écrit pour la première fois, alors je suis d'accord avec "ne pas comprendre les booléens"
Izkata
13

Chaque fois que le code est plus compliqué qu'il ne doit l'être, il déclenche un "qu'est-ce que c'est censé faire?" sentir dans le lecteur.

Par exemple, le premier exemple me fait me demander: "Y avait-il d'autres fonctionnalités dans l'instruction if / else à un moment donné qui ont été supprimées?"

L'exemple (2) est simple, clair et fait exactement ce qui est nécessaire. Je l'ai lu et je comprends immédiatement ce que fait le code.

Le fluff supplémentaire dans (3) me ferait me demander pourquoi l'auteur l'a écrit de cette façon au lieu de (2). Il devrait y avoir une raison, mais dans ce cas, il ne semble pas y en avoir, donc ce n'est pas utile du tout et plus difficile à lire car la syntaxe suggère quelque chose de présent qui n'est pas là. Essayer d'apprendre ce qui est présent (quand il n'y a rien) rend le code plus difficile à lire.

enderland
la source
2

Il est facile de voir que la variante 2 et la variante 1 sont liées via une série de refactorisations simples et évidentes:

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.NeedsChange = false;
}

Ici, nous avons une duplication de code inutile, nous pouvons factoriser l'affectation:

obj.NeedsChange = if (obj.Performance <= LOW_PERFORMANCE)
{
    true
}
else
{
    false
}

ou écrit de façon plus concise:

obj.NeedsChange = if (obj.Performance <= LOW_PERFORMANCE) true else false

Maintenant, il devrait être immédiatement évident que cela affectera vrai si la condition est vraie et affectera faux si la condition est fausse, IOW il assignera simplement la valeur de la condition, c'est-à-dire qu'il est équivalent à

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE

Les variantes 1 et 3 sont du code de recrue typique écrit par quelqu'un qui ne comprend pas la valeur de retour d'une comparaison.

Jörg W Mittag
la source
J'ajouterais votre si (...) ... fausse partie en commentaire juste avant la belle, alors vous obtenez le scan simple à travers la clarté du code et le meilleur code.
DaveM
2

Alors que votre programmation devrait tendre à être explicite plutôt qu'implicite "comme si le gars qui finit par maintenir votre code sera un psychopathe violent qui sait où vous vivez", vous pouvez supposer quelques choses de base dans lesquelles votre successeur psycho sera compétent.

L'un d'eux est la syntaxe du langage qu'il utilise.

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE;

est très clair pour quiconque connaît la syntaxe C / C ++ / C # / Java / Javascript.

Il est également beaucoup plus lisible que 8 lignes.

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.NeedsChange = false;
}

et moins sujet aux erreurs

if (obj.Performance <= LOW_PERFORMANCE)
{
    obj.NeedsChange = true;
}
else
{
    obj.Needschange = false;
}

Et c'est mieux que d'ajouter des caractères inutiles, comme si vous aviez à moitié oublié la syntaxe de la langue:

obj.NeedsChange = obj.Performance <= LOW_PERFORMANCE ? true : false;

obj.NeedsChange = (obj.Performance <= LOW_PERFORMANCE);

Je pense qu'il y a beaucoup de mal dans la syntaxe de type C de nombreux langages: ordre d'opérations incohérent, associativité gauche / droite incohérente, utilisations surchargées de symboles, duplication d'accolades / indentation, opérateurs ternaires, notation infixe, etc.

Mais la solution n'est pas d'en inventer votre propre version propriétaire. De cette façon réside la folie, car chacun crée le sien.


Généralement, ce qui rend le code Real World TM illisible, c'est sa quantité.

Entre un programme de 200 lignes non pathologique et un programme de 1 600 lignes trivialement identique, le plus court sera presque toujours plus facile à analyser et à comprendre. J'accueillerais votre changement n'importe quel jour.

Paul Draper
la source
1

La plupart des développeurs seront capables de comprendre la 2ème forme d'un coup d'œil. À mon avis, la simplification comme dans la première forme est tout simplement inutile.

Vous pouvez améliorer la lisibilité en ajoutant des espaces et des accolades tels que:

obj.NeedsChange =    obj.Performance <= LOW_PERFORMANCE;

ou

obj.NeedsChange = ( obj.Performance <= LOW_PERFORMANCE );

comme mentionné par Jacob Raihle.

Uday Shankar
la source