Le développeur insiste sur le fait que les déclarations ne doivent pas avoir des conditions négatives, mais doivent toujours avoir un bloc else

169

J'ai une connaissance, un développeur plus expérimenté que moi. Nous parlions de pratiques de programmation et son approche des déclarations «if» m'a surpris. Il insiste sur certaines pratiques concernant les déclarations que je trouve plutôt étranges.

Premièrement , une déclaration if devrait être suivie d'une déclaration else, qu'il y ait quelque chose à ajouter ou non. Ce qui conduit à un code ressemblant à ceci:

if(condition) 
{
    doStuff();
    return whatever;
}
else
{
}

Deuxièmement , il est préférable de tester les vraies valeurs plutôt que les fausses. Cela signifie qu'il est préférable de tester une variable 'doorClosed' au lieu d'une variable '! DoorOpened'

Son argument est que cela clarifie ce que fait le code.

Ce qui me trouble un peu, car la combinaison de ces deux règles peut l'amener à écrire ce type de code s'il veut faire quelque chose lorsque la condition n'est pas remplie.

if(condition)
{
}
else
{
    doStuff();
    return whatever;
}

Mon sentiment à ce sujet est qu’il est en effet très moche et / ou que l’amélioration de la qualité, s’il y en a une, est négligeable. Mais en tant que junior, je suis susceptible de douter de mon instinct.

Mes questions sont donc les suivantes: est-ce une pratique bonne / mauvaise / "peu importe"? Est-ce une pratique courante?

Patsuan
la source
57
Est-il possible que votre collègue vienne d'un milieu où la vie et l'intégrité physique sont en jeu? Je pense avoir vu quelques exemples de ce genre de choses dans des systèmes où une tonne de tests et d'analyses automatiques est faite sur du code et où se tromper pourrait littéralement coûter la vie à quelqu'un.
jpmc26
11
Que dit le guide de style de votre entreprise?
Thorbjørn Ravn Andersen
4
Réponse partielle à votre question "Secondly". Si l'un des blocs d'action est long et l'autre court, testez une condition qui met le bloc court en premier, de sorte qu'il soit moins susceptible d'être manqué lors de la lecture du code. Cette condition peut être une négation ou un changement de nom pour en faire un test de vrai.
Ethan Bolker
9
Resharper aurait quelque chose à dire à votre ami, dans la ligne suivante: "Votre code est redondant et inutile, voulez-vous que je le supprime / le remanie?"
BgrWorker

Réponses:

184

elseBloc explicite

La première règle ne fait que polluer le code et ne le rend pas plus lisible, ni moins sujet aux erreurs. Je suppose que l'objectif de votre collègue est d'être explicite en montrant que le développeur était parfaitement conscient du fait que la condition peut être évaluée à false. Bien que ce soit une bonne chose d'être explicite, une telle explicité ne devrait pas coûter trois lignes de code supplémentaires .

Je ne mentionne même pas le fait qu’une ifdéclaration n’est pas nécessairement suivie d’un elseou de rien: elle peut aussi être suivie d’un ou de plusieurs elifmots.

La présence de la returndéclaration aggrave les choses. Même si vous aviez du code à exécuter dans le elsebloc, il serait plus facile de le faire comme ceci:

if (something)
{
    doStuff();
    return whatever;
}

doOtherThings();
return somethingElse;

Cela fait que le code prenne deux lignes de moins et annule le elseblocage. Les clauses de garde sont toutes à ce sujet.

Notez cependant que la technique de votre collègue pourrait résoudre partiellement un très vilain schéma de blocs conditionnels empilés sans espaces:

if (something)
{
}
if (other)
{
}
else
{
}

Dans le code précédent, le manque de saut de ligne sain après le premier ifbloc rend très facile l’interprétation erronée du code. Cependant, alors que la règle de votre collègue rendrait plus difficile la mauvaise interprétation du code, une solution plus simple consisterait simplement à ajouter une nouvelle ligne.

Tester pour true, pas pourfalse

La deuxième règle pourrait avoir un sens, mais pas dans sa forme actuelle.

Il n'est pas faux de dire que le test d'une porte fermée est plus intuitif que celui d'une porte non ouverte . Les négations, et en particulier les négations imbriquées, sont généralement difficiles à comprendre:

if (!this.IsMaster || (!this.Ready && !this.CanWrite))

Pour résoudre ce problème, au lieu d’ajouter des blocs vides, créez des propriétés supplémentaires, le cas échéant, ou des variables locales .

La condition ci-dessus pourrait être rendue lisible assez facilement:

if (this.IsSlave || (this.Synchronizing && this.ReadOnly))
Arseni Mourzenko
la source
169
Les blocs vides ressemblent toujours à des erreurs pour moi. Cela attirera immédiatement mon attention et je demanderai: "Pourquoi est-ce ici?"
Nelson
50
@Neil La négation d'une condition ne nécessite pas nécessairement de temps processeur supplémentaire. C compilé en x86 pourrait simplement échanger l’instruction de saut de JZ(Sauter si zéro) if (foo)en JNZ(Sauter si non nul) pour if (!foo).
8bittree
71
" créer des propriétés supplémentaires avec des noms clairs ": à moins que vous n'ayez une analyse plus approfondie du domaine, cela peut modifier la portée globale (la classe d'origine) pour résoudre un problème local (l'application d'une règle de gestion spécifique). Une chose à faire ici est de créer des booléens locaux avec l'état souhaité, tels que "var isSlave =! This.IsMaster" et d'appliquer votre if avec les booléens locaux au lieu de créer plusieurs autres propriétés. Alors, prenez le conseil avec un grain de sel et prenez le temps d'analyser le domaine avant de créer de nouvelles propriétés.
Machado
82
Il ne s'agit pas de "trois lignes de code", il s'agit d'écrire pour le public approprié (quelqu'un avec au moins une connaissance de base de la programmation). An ifest déjà explicite sur le fait que la condition pourrait être fausse ou que vous ne la testeriez pas. Un bloc vide rend les choses moins claires, pas plus, parce qu'aucun lecteur n'est prêt à l'attendre, et maintenant, ils doivent s'arrêter et réfléchir à la façon dont cela aurait pu arriver à cela.
Hobbs
15
@Marque c'est moins clair, même avec le commentaire, que de dire if (!commonCase) { handle the uncommon case },cependant.
Paul
62

En ce qui concerne la première règle, il s’agit d’un exemple de saisie inutile. Non seulement cela prend plus de temps à taper, cela causera une énorme confusion à quiconque lira le code. Si le code n'est pas nécessaire, ne l'écrivez pas. Cela s’étendrait même à ne pas avoir un cas rempli elsedans votre cas lorsque le code revient du ifbloc:

if(condition) 
{
    doStuff();
    return whatever;
}

doSomethingElse(); // no else needed
return somethingelse;

En ce qui concerne le deuxième point, il convient d’éviter les noms booléens contenant un négatif:

bool doorNotOpen = false; // a double negative is hard to reason
bool doorClosed = false; // this is much clearer

Toutefois, comme vous l'avez souligné, le fait d'étendre cette procédure à l'absence de contrôle négatif d'un test négatif conduit à une saisie plus inutile. Ce qui suit est bien plus clair que d'avoir un ifbloc vide :

if(!condition)
{
    doStuff();
    return whatever;
}
David Arno
la source
120
Alors ... vous pourriez dire que la double négatif est un non-non? ;)
Patsuan
6
@Patsuan, très intelligent. J'aime ça. :)
David Arno
4
Je pense que peu de gens seraient d’accord sur le point de savoir comment traiter les retours, et certains pourraient même dire que cela ne devrait pas être le même dans tous les cas. Je n'ai pas de préférence personnelle moi-même, mais je peux certainement voir l'argument derrière de nombreuses autres façons de le faire.
Dukeling
6
Clairement, if (!doorNotOpen)c'est affreux. Les noms doivent donc être aussi positifs que possible. if (! doorClosed)est parfaitement bien.
David Schwartz
1
En ce qui concerne votre exemple de porte, les noms des deux exemples ne sont pas nécessairement équivalents. Dans le monde physique, ce doorNotOpenn'est pas la même chose car doorClosedil y a un état de transition entre être ouvert et fermé. Je le vois tout le temps dans mes applications industrielles où les états booléens sont déterminés par des capteurs physiques. Si vous avez un seul capteur sur le côté ouvert de la porte, vous pouvez seulement dire que la porte est ouverte ou non ouverte. De même, si le capteur est du côté fermé, vous pouvez uniquement dire fermé ou non fermé. De plus, le capteur lui-même détermine comment l'état logique est affirmé.
Peter M
45

1. Un argument en faveur des elsedéclarations vides .

J'utilise souvent (et plaide pour) quelque chose qui ressemble à cette première construction, une autre vide. Cela indique aux lecteurs du code (outils d'analyse humains et automatisés) que le programmeur a réfléchi à la situation. Les elsedéclarations manquantes qui auraient dû être présentes ont tué des personnes, fait tomber des véhicules et coûté des millions de dollars. MISRA-C, par exemple, impose au moins un commentaire indiquant que la dernière finale manquante est intentionnelle dans une if (condition_1) {do_this;} else if (condition_2) {do_that;} ... else if (condition_n) {do_something_else;}séquence. D'autres normes de haute fiabilité vont encore plus loin: à quelques exceptions près, les autres déclarations manquantes sont interdites.

Une exception est un simple commentaire, quelque chose du genre /* Else not required */. Cela signale la même intention que les trois lignes vides. Une autre exception où ce vide vide n'est pas nécessaire est celle où cela est flagrant pour les lecteurs du code et pour les outils d'analyse automatisés que ce vide vide est superflu. Par exemple, de if (condition) { do_stuff; return; }même, un caractère vide n'est pas nécessaire dans le cas de throw somethingou goto some_label1 au lieu du return.

2. Un argument pour préférer if (condition)plus if (!condition).

Ceci est un élément de facteurs humains. La logique booléenne complexe attire beaucoup de monde. Même un programmeur expérimenté devra réfléchir if (!(complex || (boolean && condition))) do_that; else do_this;. Au minimum, réécrivez cela en tant que if (complex || (boolean && condition)) do_this; else do_that;.

3. Cela ne signifie pas qu'il faille préférer les thendéclarations vides .

La deuxième section dit "préfère" plutôt que "tu vas". C'est une ligne directrice plutôt qu'une règle. La raison pour laquelle cette directive privilégie les ifconditions positives est que le code doit être clair et évident. Une clause alors vide (par exemple, if (condition) ; else do_something;) viole cela. C'est une programmation obscurcie, ce qui amène même les programmeurs les plus aguerris à sauvegarder et à relire la ifsituation sous sa forme annulée. Donc, écrivez-le sous la forme refusée en premier lieu et omettez la déclaration else (ou ayez un caractère vide ou un commentaire à cet effet s'il est mandaté pour le faire).



1 J'ai écrit qu'alors les clauses qui se terminent par return, throwou goton'exigent pas de vide. Il est évident que la clause else n'est pas nécessaire. Mais qu'en est-il goto? En passant, les règles de programmation critiques pour la sécurité interdisent parfois les retours anticipés, et interdisent presque toujours les exceptions de lancement. Ils permettent cependant gotosous une forme restreinte (par exemple goto cleanup1;). Cette utilisation restreinte de gotoest la pratique préférée dans certains endroits. Le noyau Linux, par exemple, est plein à craquer de telles gotodéclarations.

David Hammen
la source
6
!aussi est facilement négligé. C'est trop laconique.
Thorbjørn Ravn Andersen
4
Non, un autre vide n'indique pas clairement que le programmeur y a réfléchi. Cela conduit à plus de confusion "Y avait-il des déclarations prévues et ils ont oublié ou pourquoi y a-t-il une clause vide?" Un commentaire est beaucoup plus clair.
Simon
9
@Simon: Une forme typique est else { /* Intentionally empty */ }, ou quelque chose du genre . Le paramètre vide satisfait l'analyseur statique qui recherche sans scrupule les violations des règles. Le commentaire informe les lecteurs que le reste vide est intentionnel. Là encore, les programmeurs chevronnés omettent généralement le commentaire car inutile - mais pas le vide. La programmation haute fiabilité est un domaine à part. Les constructions semblent plutôt étranges pour les étrangers. Ces constructions sont utilisées en raison des leçons tirées de l'école des coups durs, des coups très durs lorsque la vie et la mort ou des $$$$$$$$$$$$$$$$$$$$$$$
David Hammen
2
Je ne comprends pas à quel point les clauses vides sont une mauvaise chose, alors que les clauses vides else ne le sont pas (en supposant que nous choisissions ce style). Je peux voirif (target == source) then /* we need to do nothing */ else updateTarget(source)
Bergi
2
@ ThorbjørnRavnAndersen Nope, notest un mot clé en C ++, au même titre que les autres opérateurs binaires et logiques. Voir les représentations alternatives des opérateurs .
Ruslan
31

J'utilise une branche else vide (et parfois une branche if vide) dans de très rares cas: lorsqu'il est évident que les parties if et else doivent être traitées d'une manière ou d'une autre, mais que, pour une raison quelconque, le cas peut être traité par Ne rien faire. Et par conséquent, toute personne lisant le code avec l’action else nécessaire suspecterait immédiatement qu’il manquait quelque chose et perdrait son temps.

if (condition) {
    // No action needed because ...
} else {
    do_else_action()
}

if (condition) {
    do_if_action()
} else {
    // No action needed because ...
}

Mais non:

if (condition) {
    do_if_action()
} else {
    // I was told that an if always should have an else ...
}
gnasher729
la source
5
Cette. Je trouve la déclaration if test succeeds -> no action needed -> else -> action neededsémantiquement très différente de la déclaration if it applies that the opposite of a test outcome is true then an action is needed. Je me souviens de la citation de Martin Fowler: "Tout le monde peut écrire du code que les ordinateurs peuvent comprendre; les bons programmeurs écrivent du code que les humains peuvent comprendre".
Tasos Papastylianou
2
@TasosPapastylianou C'est de la triche. Le contraire de "si le test réussit -> aucune action nécessaire" est "si le test échoue -> une action nécessaire". Vous l'avez rempli avec environ 10 mots.
Jerry B
@JerryB cela dépend du "test". Parfois, la négation est simple (sur le plan linguistique), mais parfois non, et créerait de la confusion. Dans ces cas, il est plus clair de parler de "négation / opposé du résultat qui est vrai" ou de "résultat qui n'est pas vrai"; Cependant, le fait est qu'il serait encore plus clair d'introduire une étape 'vide' ou d'inverser le sens des noms de variables. Ceci est typique dans les situations "de morgan": par exemple, if ((missing || corrupt) && !backup) -> skip -> else do stuffest beaucoup plus clair (+ intention implicite différente) queif ((!missing && !corrupt) || backup) -> do stuff
Tasos Papastylianou
1
@TasosPapastylianou Vous pouvez écrire if(!((missing || corrupt) && !backup)) -> do stuffou mieux if((aviable && valid) || backup) -> do stuff. (Mais dans un exemple réel, vous devez vérifier si la sauvegarde est valide avant de l'utiliser)
12431234123412341234123
2
@TasosPapastylianou Où y a-t-il des doubles négatifs dans ce que j'ai dit? Bien sûr, si vous utilisiez un nom de variable non corrompu, vous auriez un point clair. Bien que lorsque vous rencontrez de tels opérateurs booléens, il peut être préférable d’utiliser des variables temporaires dans le if: file_ok = !missing && !corrupt; if(file_ok || backup) do_stuff();ce qui, j’estime personnellement, le rend encore plus clair.
Baldrickk
23

Toutes choses égales par ailleurs, préférez la brièveté.

Ce que vous n'écrivez pas, personne ne doit lire et comprendre.

Bien qu'explicite puisse être utile, ce n'est le cas que s'il est évident, sans trop de verbosité, que ce que vous avez écrit est vraiment ce que vous vouliez écrire.


Donc, évitez les branches vides, elles sont non seulement inutiles, mais aussi rares et créent donc la confusion.

De même, évitez d'écrire une autre branche si vous quittez directement la branche if.

Une application utile de la clarté consisterait à mettre un commentaire chaque fois que vous tombez dans le casse // FALLTHRU, et à utiliser un commentaire ou un bloc vide dans lequel vous avez besoin d'une instruction vide for(a;b;c) /**/;.

Déduplicateur
la source
7
// FALLTHRUUgh, pas dans SCREAMING CAPS ou avec les abréviations de txtspk. Sinon, je suis d'accord et suis cette pratique moi-même.
Cody Gray
7
@CodyGray - C’est l’un des endroits où crier est une bonne idée. La plupart des états de commutation de fin de chaque cas break. Cela breaka conduit à des pannes logicielles spectaculaires. Un commentaire qui crie aux lecteurs du code que la chute est intentionnelle est une bonne chose. Le fait que les outils d'analyse statiques puissent rechercher ce modèle spécifique et ne pas se plaindre d'une situation de chute en font une chose encore meilleure.
David Hammen
1
@Wossname: Je viens de vérifier mon nom vimet il ne reconnaît aucune variation de FALLTHRU. Et je pense que, pour une bonne raison: TODOet FIXMEcommentaires sont les commentaires qui doivent être grepable parce que vous voulez être en mesure de demander à vos git grepdéfauts qui vous avez connus dans votre code, et où ils se trouvent. Une chute n'est pas un défaut, et il n'y a aucune raison de la réparer, quoi qu'il en soit.
cmaster
4
@DavidHammen Non, crier n'est pas une bonne idée: si vous avez un // échec qui remplace une pause attendue, cela devrait être suffisant pour que tout développeur comprenne immédiatement. En outre, // fallthrough ne parle pas d'un défaut, il indique simplement que les circonstances ont été prises en compte et que la pause a été effectuée. qui semble manquer, est en effet destiné à ne pas être là. Ceci est un but purement informatif, et rien à dire.
cmaster
1
@cmaster - Votre opinion est que crier n'est pas une bonne idée . Les opinions des autres sont différentes. Et ce n'est pas parce que votre configuration de vim ne reconnaît FALLTHRUpas que d'autres personnes n'ont pas configuré vim pour le faire.
David Hammen
6

À ma connaissance, il n’existe pas de règle absolue concernant les conditions positives ou négatives d’un énoncé IF. Personnellement, je préfère coder pour un cas positif que pour un cas négatif, le cas échéant. Je ne le ferai certainement pas, cependant, si cela me conduit à créer un bloc SI vide, suivi d'un autre élément logique. Si une telle situation se produisait, cela prendrait environ 3 secondes pour le refactoriser à nouveau pour rechercher un cas positif de toute façon.

Ce que je n'aime pas vraiment dans vos exemples, c’est l’espace vertical totalement inutile occupé par le ELSE vierge. Il n'y a tout simplement aucune raison de le faire. Cela n'ajoute rien à la logique, cela n'aide pas à documenter ce que fait le code, et cela n'augmente pas la lisibilité. En fait, je dirais que l’espace vertical ajouté pourrait éventuellement diminuer la lisibilité.

Langecrew
la source
2
Cet espace vertical inutile est une conséquence du mandat consistant à toujours utiliser des accolades, à interdire les éléments manquants et à utiliser le style Allman pour l'indentation.
David Hammen
Eh bien, je pense que le fait d’utiliser des accolades est une bonne chose, car les intentions du développeur sont explicites - il n’ya aucune place pour des suppositions lorsque vous relisez leur code. Cela peut paraître idiot, mais croyez-moi, en particulier lorsque vous encadrez des développeurs débutants, des erreurs liées à des accolades manquantes sont commises. Personnellement, je n'ai jamais été fan de l'indentation de style Allman, pour la raison que vous mentionnez: espace vertical inutile. Mais ce n'est que mon opinion et mon style personnel. C'est juste ce que j'ai appris au départ, donc je me suis toujours senti plus à l'aise pour lire.
Langecrew
Style personnel: quel que soit le style que j'utilise (Allman, 1 To, ...), j'utilise toujours des accolades.
David Hammen
Les conditions complexes ou celles que vous pensez difficiles à comprendre peuvent presque toujours être résolues en étant redéfinies dans leurs propres méthodes / fonctions. Quelque chose comme cela if(hasWriteAccess(user,file))pourrait être complexe en dessous, mais en un coup d'œil, vous savez exactement quel devrait être le résultat. Même s'il ne s'agit que de deux conditions, par exemple, if(user.hasPermission() && !file.isLocked())un appel de méthode avec un nom approprié permet de faire passer le message, de sorte que les aspects négatifs deviennent moins problématiques.
Chris Schneider
D'accord. Encore une fois, je suis un grand fan de refactoring autant que possible :)
Langecrew le
5

elseBloc explicite

Je ne suis pas d’accord avec cette affirmation en tant que déclaration générale couvrant toutes les ifdéclarations, mais il arrive que l’ajout d’un elsebloc par habitude soit une bonne chose.

Une ifdéclaration, à mon avis, recouvre en réalité deux fonctions distinctes.

Si nous sommes censés faire quelque chose, faites-le ici.

Ce genre de choses n'a évidemment pas besoin d'une elsepartie.

    if (customer.hasCataracts()) {
        appointmentSuggestions.add(new CataractAppointment(customer));
    }
    if (customer.isDiabetic()) {
        customer.assignNurse(DiabeticNurses.pickBestFor(customer));
    }

et dans certains cas insister pour ajouter un elsepourrait induire en erreur.

    if (k > n) {
        return BigInteger.ZERO;
    }
    if (k <= 0 || k == n) {
        return BigInteger.ONE;
    }

n'est pas la même chose que

    if (k > n) {
        return BigInteger.ZERO;
    } else {
        if (k <= 0 || k == n) {
            return BigInteger.ONE;
        }
    }

même si c'est fonctionnellement le même. Écrire le premier ifavec un vide elsepeut vous mener au deuxième résultat qui est inutilement laid.

Si nous recherchons un état spécifique, il est souvent judicieux d’ajouter un vide elseafin de vous rappeler de couvrir cette éventualité.

        // Count wins/losses.
        if (doors[firstChoice] == Prize.Car) {
            // We would have won without switching!
            winWhenNotSwitched += 1;
        } else {
            // We win if we switched to the car!
            if (doors[secondChoice] == Prize.Car) {
                // We picked right!
                winWhenSwitched += 1;
            } else {
                // Bad choice.
                lost += 1;
            }
        }

N'oubliez pas que ces règles s'appliquent uniquement lorsque vous écrivez un nouveau code . IMHO Les elseclauses vides doivent être supprimées avant l'archivage.


Tester pour true, pas pourfalse

Là encore, c’est un bon conseil au niveau général, mais dans de nombreux cas, cela rend le code inutilement complexe et moins lisible.

Même si le code comme

    if(!customer.canBuyAlcohol()) {
        // ...
    }

est choquant pour le lecteur, mais en le rendant

    if(customer.canBuyAlcohol()) {
        // Do nothing.
    } else {
        // ...
    }

est au moins aussi mauvais, sinon pire.

J'ai codé dans BCPL il y a de nombreuses années et dans cette langue, il y a une IFclause et une UNLESSclause afin que vous puissiez coder beaucoup plus facilement comme suit:

    unless(customer.canBuyAlcohol()) {
        // ...
    }

ce qui est nettement meilleur, mais toujours pas parfait.


Mon processus personnel

Généralement, lorsque j'écris un nouveau code, j'ajoute souvent un elsebloc vide à une ifdéclaration simplement pour me rappeler que je n'ai pas encore couvert cette éventualité. Cela m'aide à éviter le DFSpiège et à s'assurer que lorsque je relis le code, je constate qu'il reste encore beaucoup à faire. Cependant, j'ajoute généralement un TODOcommentaire pour garder une trace.

  if (returnVal == JFileChooser.APPROVE_OPTION) {
    handleFileChosen();
  } else {
    // TODO: Handle case where they pressed Cancel.
  }

Je trouve que généralement j'utilise elserarement dans mon code car il peut souvent indiquer une odeur de code.

Vieux curcuma
la source
Votre traitement des blocs "vides" se lit comme un code de stubbing standard lors de la mise en oeuvre de ...
Déduplicateur
Le unlessbloc est une bonne suggestion, et à peine confiné à BCPL.
tchrist
@TobySpeight Oups. J'ai en quelque sorte échappé à mon attention que ce que j'ai écrit ...était en réalité return. (En fait, avec k=-1, n=-2le premier retour est pris mais c'est en grande partie théorique.)
David Richerby le
Perl moderne a ifet unless. Qui plus est, cela permet également de les suivre après une déclaration, vous pouvez donc dire print "Hello world!" if $born;ou les print "Hello world!" unless $dead;deux feront exactement ce que vous pensez qu'ils font.
un CVn
1

Pour le premier point, j’ai utilisé un langage qui forçait les déclarations IF à être utilisé de cette façon (dans Opal, le langage situé derrière un grattoir d’écran principal pour placer une interface graphique sur les systèmes mainframe) et avec une seule ligne pour le SI. et les autres. Ce n'était pas une expérience agréable!

Je m'attendrais à ce que tout compilateur optimise ces clauses ELSE supplémentaires. Mais pour le code live, cela n’ajoute rien (en développement, cela peut être un marqueur utile pour le code suivant).

Une fois que j'utilise quelque chose comme ces clauses supplémentaires, c'est lors de l'utilisation du traitement de type CASE / WHEN. J'ajoute toujours une clause par défaut même si elle est vide. C’est une habitude à long terme de la part de langages qui vont se tromper si une telle clause n’est pas utilisée, et oblige à se demander si les choses doivent vraiment passer à travers.

Il y a longtemps, la pratique des ordinateurs centraux (par exemple, PL / 1 et COBOL) supposait que les contrôles négatifs étaient moins efficaces. Cela pourrait expliquer le deuxième point, bien que de nos jours, il existe des économies d’efficacité considérablement plus importantes qui sont ignorées sous forme de micro optimisations.

La logique négative a tendance à être moins lisible, mais pas tellement sur une déclaration IF aussi simple.

Kickstart
la source
2
Je vois, donc il était pratique courante à un moment donné.
Patsuan
@Patsuan - oui, dans une certaine mesure. Bien que c’était à ce moment-là que l’assembleur de mainframe était une alternative sérieuse au code critique de performance.
Kickstart le
1
"Il y a longtemps ... il a été accepté que les contrôles négatifs étaient moins efficaces." Eh bien, ils sont à moins que le compilateur optimise if !A then B; else Cpour if A then C; else B. Le calcul de la négation de la condition est une instruction supplémentaire de l’UC. (Bien que, comme vous le dites, il est peu probable que cela soit nettement moins efficace.)
David Richerby Le
@DavidRicherby Et si nous parlons en général, certaines langues peuvent rendre de telles optimisations très difficiles. Prenez C ++ avec surchargé !et ==opérateurs, par exemple. Remarquez que personne ne devrait réellement le faire .
CVn
1

Je souscris à la plupart des réponses selon lesquelles les elseblocs vides sont pratiquement toujours un gaspillage d’encre électronique. N'ajoutez pas ceci à moins que vous n'ayez une très bonne raison de le faire. Dans ce cas, le bloc vide ne devrait pas être vide du tout, il devrait contenir un commentaire expliquant pourquoi il est là.

Cependant, le problème de l’évitement des négatifs mérite plus d’attention: en règle générale, chaque fois que vous devez utiliser une valeur booléenne, vous avez besoin de blocs de code pour pouvoir fonctionner lorsqu’il est défini, et d’autres pour fonctionner s’ils ne le sont pas. En tant que tel, si vous appliquez une règle de non-négatif, vous appliquez soit des if() {} else {...}déclarations ayant (avec un ifbloc vide !), Soit vous créez un deuxième booléen pour chaque booléen contenant sa valeur annulée. Les deux options sont mauvaises, car elles déroutent vos lecteurs.

Une politique utile est la suivante: n'utilisez jamais un formulaire avec négation dans le nom d'un booléen et exprimez la négation en tant que simple !. Une déclaration comme if(!doorLocked)est parfaitement claire, une déclaration comme des if(!doorUnlocked)nœuds cerveaux. Le dernier type d’expression est ce que vous devez éviter à tout prix, pas la présence d’un seul !dans une if()condition.

cmaster
la source
0

Je dirais que c'est définitivement une mauvaise pratique. Ajouter d’autres instructions va ajouter à votre code toute une série de lignes inutiles qui ne font rien et le rendent encore plus difficile à lire.

Ayrton Clark
la source
1
J'ai entendu dire que vous n'aviez jamais travaillé pour un employeur qui évalue votre performance en fonction des SLOC produits par période ...
a CVn le
0

Il y a un autre motif qui n'a pas encore été mentionné concernant le traitement du deuxième cas qui a un bloc if vide. Une façon de régler ce problème serait de revenir dans le bloc if. Comme ça:

void foo()
{
    if (condition) return;

    doStuff();
    ...
}

C'est un modèle courant pour vérifier les conditions d'erreur. Le cas affirmatif est toujours utilisé et son contenu n'est plus vide, laissant les futurs programmeurs se demander si un non-op était intentionnel ou non. Cela peut également améliorer la lisibilité, car il se peut que vous deviez créer une nouvelle fonction (divisant ainsi de grandes fonctions en fonctions individuelles plus petites).

Shaz
la source
0

Il y a un point en considérant l'argument "toujours avoir une clause else" que je n'ai vu dans aucune autre réponse: cela peut avoir un sens dans un style de programmation fonctionnel. Sorte de.

Dans un style de programmation fonctionnel, vous manipulez des expressions plutôt que des instructions. Ainsi, chaque bloc de code a une valeur de retour, y compris une if-then-elseexpression. Cela empêcherait toutefois un elsebloc vide . Laissez-moi vous donner un exemple:

var even = if (n % 2 == 0) {
  return "even";
} else {
  return "odd";
}

Maintenant, dans les langages avec un style C ou une syntaxe inspirée du style C (comme Java, C # et JavaScript, pour n'en nommer que quelques-uns), cela a l'air bizarre. Cependant, il semble beaucoup plus familier lorsqu'il est écrit en tant que tel:

var even = (n % 2 == 0) ? "even" : "odd";

Si vous laissez la elsebranche vide ici, une valeur ne sera pas définie - dans la plupart des cas, ce n'est pas ce que nous voulons être un cas valide lors de la programmation de fonctionnalités. Même avec le laisser complètement. Cependant, lorsque vous programmez de manière itérative, je vous donne très peu de raisons de toujours avoir un elsebloc.

Blalasaadri
la source
0

... une déclaration if devrait être suivie d'une déclaration else, qu'il y ait quelque chose à ajouter ou non.

Je suis en désaccord if (a) { } else { b(); }devrait être ré-écrit comme if (!a) { b(); }et if (a) { b(); } else { }devrait être ré-écrit comme if (a) { b(); }.

Toutefois, il convient de souligner que j'ai rarement une branche vide. C'est parce que normalement je me connecte que je suis entré dans la branche autrement vide. De cette façon, je peux développer uniquement des messages de journalisation. J'utilise rarement un débogueur. Dans les environnements de production, vous ne recevez pas de débogueur; il est agréable de pouvoir résoudre les problèmes de production avec les mêmes outils que ceux que vous utilisez pour développer.

Deuxièmement, il est préférable de tester les vraies valeurs plutôt que les fausses. Cela signifie qu'il est préférable de tester une variable 'doorClosed' au lieu d'une variable '! DoorOpened'

J'ai des sentiments mitigés à ce sujet. Un inconvénient d'avoir doorClosedet doorOpenedest - il peut doubler le nombre de mots / termes que vous devez être au courant. Avec le temps, un autre inconvénient est la signification de doorClosedet doorOpenedpeut changer (d'autres développeurs arrivant après vous) et vous pouvez vous retrouver avec deux propriétés qui ne sont plus des négations précises l'une de l'autre. Au lieu d'éviter les négations, j'apprécie d'adapter la langue du code (noms de classes, noms de variables, etc.) au vocabulaire des utilisateurs professionnels et aux exigences qui me sont données. Je ne voudrais pas inventer un nouveau terme pour éviter un!si ce terme n'a de signification pour un développeur que les utilisateurs professionnels ne comprendront pas. Je souhaite que les développeurs parlent le même langage que les utilisateurs et les personnes qui rédigent les exigences. Maintenant, si de nouveaux termes simplifient le modèle, c'est important, mais vous devez le gérer avant la finalisation des exigences, pas après. Le développement devrait gérer ce problème avant de commencer à coder.

Je suis enclin à douter de mon instinct.

C'est bien de continuer à se questionner.

Est-ce une bonne / mauvaise / pratique "n'a pas d'importance"?

Dans une certaine mesure, cela n'a pas d'importance. Obtenez votre code révisé et adapter à votre équipe. C'est généralement la bonne chose à faire. Si tous les membres de votre équipe souhaitent coder d'une certaine manière, il est probablement préférable de le faire de cette façon (changer de personne - vous-même - nécessite moins de points de récit que de changer de groupe de personnes).

Est-ce une pratique courante?

Je ne me souviens pas avoir jamais vu une branche vide. Je vois des gens ajouter des propriétés pour éviter les négations, cependant.

Des mots comme Jared
la source
0

Je vais seulement aborder la deuxième partie de la question et cela vient de mon point de vue de travailler dans des systèmes industriels et de manipuler des appareils dans le monde réel.

Cependant, il s’agit en quelque sorte d’un commentaire étendu plutôt que d’une réponse.

Quand il est déclaré

Deuxièmement, il est préférable de tester les vraies valeurs plutôt que les fausses. Cela signifie qu'il est préférable de tester une variable 'doorClosed' au lieu d'une variable '! DoorOpened'

Son argument est que cela clarifie ce que fait le code.

De mon point de vue, on suppose ici que l'état de la porte est un état binaire alors qu'en fait il s'agit au moins d'un état ternaire:

  1. La porte est ouverte.
  2. La porte n'est ni complètement ouverte ni complètement fermée.
  3. La porte est fermée.

Ainsi, selon l’emplacement d’un capteur numérique binaire, vous ne pouvez conjecturer qu’un des deux concepts suivants:

  1. Si le capteur est sur le côté ouvert de la porte: la porte est ouverte ou non ouverte
  2. Si le capteur est sur le côté fermé de la porte: La porte est fermée ou non fermée.

Et vous ne pouvez pas échanger ces concepts en utilisant une négation binaire. Ainsi, doorClosedet ne !doorOpenedsont probablement pas synonymes, et toute tentative de prétendre qu’elles sont synonymes est une pensée erronée qui suppose une meilleure connaissance de l’état du système qu’elle n’existe réellement.

Pour revenir à votre question, j'appuierais un verbiage qui correspond à l'origine de l'information représentée par la variable. Ainsi, si les informations proviennent du côté fermé de la porte, allez-y avec, doorClosedetc. Cela peut impliquer d'utiliser l'une doorClosedou l' autre ou !doorClosedselon les besoins dans diverses déclarations, selon les besoins, ce qui risque de créer une confusion avec l'utilisation de la négation. Mais ce qu’elle ne fait pas, c’est propager implicitement des hypothèses sur l’état du système.


À des fins de discussion pour le travail que je fais, la quantité d'informations dont je dispose sur l'état du système dépend des fonctionnalités requises par le système dans son ensemble.

Parfois, je n'ai besoin que de savoir si la porte est fermée ou non. Dans ce cas, un seul capteur binaire suffira. Mais à d'autres moments, j'ai besoin de savoir que la porte est ouverte, fermée ou en transition. Dans ce cas, il y aurait soit deux capteurs binaires (à chaque extrémité du mouvement de la porte - la vérification des erreurs s'opérant simultanément contre l'ouverture et la fermeture de la porte), soit un capteur analogique mesurant le degré d'ouverture de la porte.

Un exemple de la première serait la porte sur un four à micro-ondes. Vous activez le fonctionnement du four lorsque la porte est fermée ou non. Vous ne vous souciez pas de la façon dont la porte est ouverte.

Un exemple du second serait un simple actionneur entraîné par moteur. Vous empêchez d'avancer le moteur lorsque l'actionneur est complètement sorti. Et vous empêchez le moteur d’être inversé lorsque l’actionneur est complètement enfoncé.

Fondamentalement, le nombre et le type de capteurs se résument à une analyse de coût des capteurs par rapport à une analyse des besoins de ce qui est nécessaire pour obtenir la fonctionnalité requise.

Peter M
la source
"De mon point de vue, on suppose ici que l'état de la porte est un état binaire alors qu'en fait il s'agit au moins d'un état ternaire:" et "vous activez le fonctionnement du four lorsque la porte est fermée ou non fermée Vous ne vous souciez pas de la façon dont la porte est ouverte. " se contredire. En outre, je ne pense pas que la question concerne les portes et les capteurs.
Sanchises
@Sanchises Les déclarations ne sont pas contradictoires car vous les avez sorties de leur contexte. La première affirmation se situe dans le contexte où deux mesures binaires opposées d’un état ternaire ne peuvent pas être échangées via une négation binaire. La deuxième affirmation se situe dans le contexte selon lequel seule une connaissance partielle d'un état ternaire peut suffire à une application pour fonctionner correctement. En ce qui concerne les portes, les PO ont indiqué que les portes étaient ouvertes et fermées. Je signale simplement une hypothèse évidente qui peut influer sur le traitement des données.
Peter M
-1

Les règles de style concrètes sont toujours mauvaises. Les directives sont bonnes, mais à la fin, il est souvent possible de clarifier des choses en faisant quelque chose qui ne les suit pas tout à fait.

Cela dit, il y a beaucoup de haine pour le vide vide "ça gaspille les lignes", "le dactylographie supplémentaire", ce qui est tout simplement mauvais. Vous pouvez argumenter en faveur du déplacement des crochets sur la même ligne si vous avez vraiment besoin d'espace vertical, mais si cela pose un problème, vous ne devriez de toute façon pas placer votre {sur une ligne séparée.

Comme mentionné précédemment, le bloc else est très utile pour montrer que vous voulez explicitement que rien ne se passe dans l'autre cas. Après avoir effectué beaucoup de programmation fonctionnelle (là où il est nécessaire d’autre chose), j’ai appris que vous devriez toujours tenir compte de l’autre lors de la rédaction du if, bien que @OldCurmudgeon mentionne qu’il existe vraiment deux cas d’utilisation différents. On devrait avoir un autre, on ne devrait pas. Malheureusement, ce n'est pas quelque chose que vous pouvez toujours dire en un coup d'œil, encore moins avec un linter, d'où le dogme "toujours mis un autre bloc".

En ce qui concerne les "non-négatifs", encore une fois, les règles absolues sont mauvaises. Avoir un vide si peut être étrange, surtout si c'est le type de si cela n'a pas besoin d'autre chose, alors écrivez tout ça plutôt qu'un! ou un == faux est mauvais. Cela dit, il y a beaucoup de cas où le négatif a du sens. Un exemple courant consisterait à mettre en cache une valeur:

static var cached = null

func getCached() {
    if !cached {
        cached = (some calculation, etc)
    }

    return cached
}

si la logique réelle (mentale / anglaise) implique un négatif, il devrait en être de même de l'instruction if.

zacaj
la source