Comment puis-je passer un appel avec un booléen plus clair? Piège Booléen

76

Comme le notent les commentaires de @ benjamin-gruenbaum, cela s'appelle le piège booléen:

Dis que j'ai une fonction comme celle-ci

UpdateRow(var item, bool externalCall);

et dans mon contrôleur, cette valeur externalCallsera toujours VRAIE. Quelle est la meilleure façon d'appeler cette fonction? J'écris d'habitude

UpdateRow(item, true);

Mais je me demande, devrais-je déclarer un booléen, juste pour indiquer ce que cette «vraie» valeur représente? Vous pouvez le savoir en regardant la déclaration de la fonction, mais c'est évidemment plus rapide et plus clair si vous venez de voir quelque chose comme

bool externalCall = true;
UpdateRow(item, externalCall);

PD: Je ne sais pas si cette question convient vraiment. Sinon, où pourrais-je obtenir plus d'informations à ce sujet?

PD2: Je n'ai tagué aucune langue parce que je pensais que c'était un problème très générique. Quoi qu'il en soit, je travaille avec c # et la réponse acceptée fonctionne pour c #

Mario Garcia
la source
10
Ce motif s'appelle également le piège booléen
Benjamin Gruenbaum
11
Si vous utilisez un langage prenant en charge les types de données algébriques, je recommanderais vivement un nouvel adt au lieu d'un booléen. data CallType = ExternalCall | InternalCalldans haskell par exemple.
Filip Haglund
6
Je suppose que Enums aurait exactement le même but. obtenir des noms pour les booléens, et un peu de sécurité de type.
Filip Haglund
2
Je ne suis pas d'accord pour dire que déclarer un booléen pour indiquer que le sens est "évidemment plus clair". Avec la première option, il est évident que vous passerez toujours pour vrai. Dans le second cas, vous devez vérifier (où est la variable définie? Sa valeur change-t-elle?). Bien sûr, ce n'est pas un problème si les deux lignes sont ensemble ... mais quelqu'un pourrait décider que l'endroit idéal pour ajouter son code est exactement entre les deux lignes. Ça arrive!
AJPerez
Déclarer un booléen n’est pas plus clair, c’est plus évident, c’est sauter une étape qui consiste à regarder la documentation de la méthode en question. Si vous connaissez la signature, il est moins clair d'ajouter une nouvelle constante.
insidesin

Réponses:

154

Il n'y a pas toujours de solution parfaite, mais vous avez le choix parmi plusieurs alternatives:

  • Utilisez des arguments nommés , s'ils sont disponibles dans votre langue. Cela fonctionne très bien et n'a pas d'inconvénients particuliers. Dans certaines langues, n'importe quel argument peut être passé sous forme d'argument nommé, par exemple updateRow(item, externalCall: true)( C # ) ou update_row(item, external_call=True)(Python).

    Votre suggestion d'utiliser une variable distincte est un moyen de simuler des arguments nommés, mais ne présente pas les avantages en termes de sécurité associés (rien ne garantit que vous avez utilisé le nom de variable correct pour cet argument).

  • Utilisez des fonctions distinctes pour votre interface publique, avec de meilleurs noms. C'est une autre façon de simuler des paramètres nommés en plaçant les valeurs du paramètre dans le nom.

    Ceci est très lisible, mais conduit à beaucoup de passe-passe pour vous, qui écrit ces fonctions. De plus, il ne peut pas gérer correctement une explosion combinatoire lorsqu'il existe plusieurs arguments booléens. Un inconvénient majeur est que les clients ne peuvent pas définir cette valeur de manière dynamique, mais doivent utiliser if / else pour appeler la fonction correcte.

  • Utilisez un enum . Le problème avec les booléens est qu'ils s'appellent "vrai" et "faux". Donc, introduisez plutôt un type avec de meilleurs noms (par exemple enum CallType { INTERNAL, EXTERNAL }). De plus, cela augmente la sécurité de type de votre programme (si votre langage implémente des énumérations en tant que types distincts). L'inconvénient des enums est qu'ils ajoutent un type à votre API visible publiquement. Cela n'a pas d'importance pour les fonctions purement internes et les enums ne présentent pas d'inconvénient majeur.

    Dans les langues sans énums, des chaînes courtes sont parfois utilisées à la place. Cela fonctionne et peut même être meilleur que les booléens crus, mais est très sensible aux fautes de frappe. La fonction devrait alors immédiatement affirmer que l'argument correspond à un ensemble de valeurs possibles.

Aucune de ces solutions n’a un impact négatif sur les performances. Les paramètres nommés et les énumérations peuvent être résolus complètement lors de la compilation (pour un langage compilé). L'utilisation de chaînes peut impliquer une comparaison de chaînes, mais son coût est négligeable pour les littéraux de chaîne de petite taille et la plupart des types d'applications.

Amon
la source
7
Je viens d'apprendre que des "arguments nommés" existent. Je pense que c'est de loin la meilleure suggestion. Si vous pouvez élaborer un peu sur cette option (pour que les autres personnes n'aient pas besoin de Google à ce sujet), j'accepterai cette réponse. Aussi, des indices sur la performance si vous le pouvez ... Merci
Mario Garcia
6
Une chose qui peut aider à atténuer les problèmes liés aux chaînes courtes est d'utiliser des constantes avec les valeurs de chaîne. @MarioGarcia Il n'y a pas grand chose à développer. Mis à part ce qui est mentionné ici, la plupart des détails dépendront de la langue. Y a-t-il quelque chose de particulier que vous vouliez voir mentionné ici?
jpmc26
8
Dans les langues où nous parlons d'une méthode et dont l'énum peut être défini comme une classe, j'utilise généralement une enum. C'est complètement auto-documentant (dans la seule ligne de code qui déclare le type enum, donc c'est léger aussi), empêche complètement l'appel de la méthode avec un booléen nu, et l'impact sur l'API publique est négligeable (puisque les identifiants sont étendus à la classe).
Davidbak
4
Un autre inconvénient de l'utilisation de chaînes dans ce rôle est que vous ne pouvez pas vérifier leur validité au moment de la compilation, contrairement aux énumérations.
Ruslan
32
Enum est le gagnant. Ce n’est pas parce qu’un paramètre ne peut avoir qu’un des deux états possibles qu’il doit automatiquement être booléen.
Pete
39

La bonne solution est de faire ce que vous suggérez, mais de le regrouper dans une mini-façade:

void updateRowExternally() {
  bool externalCall = true;
  UpdateRow(item, externalCall);
}

La lisibilité l'emporte sur la micro-optimisation. Vous pouvez vous permettre l’appel de fonction supplémentaire, bien mieux que l’effort du développeur qui doit rechercher la sémantique du drapeau booléen, même une fois.

Kilian Foth
la source
8
Cette encapsulation en vaut-elle vraiment la peine lorsque je n’appelle cette fonction qu’une seule fois dans mon contrôleur?
Mario Garcia
14
Oui. Oui, ça l'est. La raison, comme je l'ai écrit plus haut, est que le coût (un appel de fonction) est inférieur au bénéfice (vous économisez un temps non négligeable pour le développeur, car personne n'a jamais à rechercher ce qui est truefait.) coûte autant de temps processeur que de gain de temps pour le développeur, car le temps processeur est beaucoup moins cher.
Kilian Foth
8
De même, tout optimiseur compétent devrait pouvoir intégrer cela à votre place, vous ne devriez donc rien perdre à la fin.
Firedraco
33
@KilianFoth Sauf que c'est une fausse hypothèse. Un mainteneur ne besoin de savoir ce que le second paramètre fait, et pourquoi il est vrai, et presque toujours cela concerne le détail de ce que vous essayez de faire. Masquer la fonctionnalité dans les coupes de mort par mille coupes réduira la facilité de maintenance, pas l’augmenter. Je l'ai vu arriver moi-même, c'est triste à dire. Un partitionnement excessif en fonctions peut en réalité être pire qu'une énorme fonction divine.
Graham
6
Je pense que ce UpdateRow(item, true /*external call*/);serait plus propre, dans des langues qui permettent cette syntaxe de commentaire. Gonfler votre code avec une fonction supplémentaire juste pour éviter d'écrire un commentaire ne semble pas en valoir la peine pour un cas simple. Peut-être que s'il y avait beaucoup d'autres arguments pour cette fonction, et / ou un code environnant encombré + délicat, cela commencerait à faire plus appel. Mais je pense que si vous déboguez, vous finirez par vérifier la fonction wrapper pour voir ce qu'elle fait et devez vous rappeler quelles fonctions sont des wrappers minces autour d'une API de bibliothèque et qui ont en réalité une certaine logique.
Peter Cordes
25

Disons que j'ai une fonction comme UpdateRow (var item, bool externalCall);

Pourquoi avez-vous une fonction comme celle-ci?

Dans quelles circonstances l' appelleriez-vous avec l'argument externalCall défini sur des valeurs différentes?

Si l’un provient d’une application cliente externe, par exemple, et que l’autre fait partie du même programme (c’est-à-dire de modules de code différents), je dirais alors que vous devriez avoir deux méthodes différentes , une pour chaque cas, éventuellement même définie sur Interfaces

Si, toutefois, vous passez l'appel en vous basant sur une donnée provenant d'une source autre que le programme (par exemple, un fichier de configuration ou une base de données lue), la méthode de passage booléen serait plus logique.

Phill W.
la source
6
Je suis d'accord. Et juste pour marteler le point soulevé pour les autres lecteurs, une analyse peut être faite de tous les appelants, qui passeront soit vrai, soit faux, soit une variable. Si aucune de ces dernières n'est trouvée, je plaiderais pour deux méthodes distinctes (sans booléen) sur une méthode avec le booléen - c'est un argument de YAGNI que le booléen introduit une complexité inutilisée / non nécessaire. Même si certains appelants transmettent des variables, je pourrais quand même fournir les versions (booléennes) sans paramètres à utiliser pour les appelants transmettant une constante - c'est plus simple, ce qui est bien.
Erik Eidt
18

Bien que je convienne qu'il est idéal d'utiliser une fonctionnalité de langage pour renforcer la lisibilité et la sécurité des valeurs, vous pouvez également opter pour une approche pratique : les commentaires à l'heure de l'appel. Comme:

UpdateRow(item, true /* row is an external call */);

ou:

UpdateRow(item, true); // true means call is external

ou (à juste titre, comme suggéré par Frax):

UpdateRow(item, /* externalCall */true);
évêque
la source
1
Aucune raison de downvote. C'est une suggestion parfaitement valable.
Suncat2000
Appréciez le <3 @ Suncat2000!
évêque
11
Une variante (probablement préférable) consiste simplement à y placer le nom de l'argument simple, comme UpdateRow(item, /* externalCall */ true ). Le commentaire de phrase complète est beaucoup plus difficile à analyser, il s’agit essentiellement de bruit (en particulier la deuxième variante, qui est également associée de manière très vague à l’argument).
Frax
1
C'est de loin la réponse la plus appropriée. C’est précisément à quoi servent les commentaires.
Sentinel
9
Pas un grand fan de commentaires comme celui-ci. Les commentaires risquent de devenir périmés s'ils ne sont pas modifiés lors de la refactorisation ou de la modification. Sans oublier que dès que vous avez plus d'un paramètre bool param, cela devient rapidement déroutant.
Jean V.
11
  1. Vous pouvez 'nommer' vos bools. Vous trouverez ci-dessous un exemple de langage OO (où il peut être exprimé dans la classe fournissant UpdateRow()), mais le concept lui-même peut être appliqué dans n’importe quel langage:

    class Table
    {
    public:
        static const bool WithExternalCall = true;
        static const bool WithoutExternalCall = false;
    

    et sur le site de l'appel:

    UpdateRow(item, Table::WithExternalCall);
    
  2. Je pense que le premier élément est meilleur, mais il ne force pas l'utilisateur à utiliser de nouvelles variables lorsqu'il utilise la fonction. Si la sécurité du type est importante pour vous, vous pouvez créer un enumtype et le faire UpdateRow()accepter à la place d'un bool:

    UpdateRow(var item, ExternalCallAvailability ec);

  3. Vous pouvez modifier le nom de la fonction pour qu’elle reflète boolmieux la signification du paramètre. Pas très sûr mais peut-être:

    UpdateRowWithExternalCall(var item, bool externalCall)

simurg
la source
8
N'aimez pas # 3 comme maintenant si vous appelez la fonction avec externalCall=false, son nom n'a aucun sens. Le reste de votre réponse est bon.
Courses de légèreté avec Monica
D'accord. Je n'étais pas très sûr mais cela pourrait être une option viable pour une autre fonction. Je
simurg
@LightnessRacesinOrbit En effet. C'est plus sûr d'y aller UpdateRowWithUsuallyExternalButPossiblyInternalCall.
Eric Duminil
@EricDuminil: Spot on 😂
Courses de légèreté avec Monica
10

Une autre option que je n'ai pas encore lue est la suivante: Utiliser un IDE moderne.

Par exemple, IntelliJ IDEA imprime le nom de variable des variables dans la méthode que vous appelez si vous passez un littéral tel que trueou nullou username + “@company.com. Ceci est fait dans une petite police pour ne pas prendre trop de place sur votre écran et avoir un aspect très différent du code réel.

Je ne dis toujours pas que c'est une bonne idée de jeter des booléens partout. L'argument selon lequel vous lisez le code beaucoup plus souvent que vous écrivez est souvent très fort, mais dans ce cas particulier, cela dépend énormément de la technologie que vous (et vos collègues!) Utilisez pour soutenir votre travail. Avec un IDE, le problème est beaucoup moins grave qu'avec, par exemple, vim.

Exemple modifié d'une partie de ma configuration de test: entrez la description de l'image ici

Sebastiaan van den Broek
la source
5
Cela ne serait vrai que si tous les membres de votre équipe utilisaient uniquement un IDE pour lire votre code. Malgré la prévalence d'éditeurs non-IDE, vous disposez également d'outils de révision de code, d'outils de contrôle de source, de questions de débordement de pile, etc. Il est essentiel que le code reste lisible, même dans ces contextes.
Daniel Pryden
@DanielPryden bien sûr, d'où mon commentaire 'et celui de vos collègues. Il est courant d'avoir un IDE standard dans les entreprises. Pour ce qui est des autres outils, je dirais qu'il est tout à fait possible que ces types d'outils prennent également en charge ceci ou cela à l'avenir, ou que ces arguments ne s'appliquent tout simplement pas (comme pour une équipe de programmation composée de deux personnes)
Sebastiaan van den Broek,
2
@Sebastiaan: Je ne pense pas être d'accord. Même en tant que développeur solo, je lis régulièrement mon propre code dans Git diffs. Git ne sera jamais capable de faire le même type de visualisation sensible au contexte qu'un IDE peut faire, et ne devrait pas le faire. Je suis tout à fait favorable à l'utilisation d'un bon IDE pour vous aider à écrire du code, mais vous ne devriez jamais utiliser un IDE comme excuse pour écrire du code que vous n'auriez pas écrit sans.
Daniel Pryden
1
@DanielPryden Je ne préconise pas l'écriture de code extrêmement bâclé. Ce n'est pas une situation en noir et blanc. Dans certains cas, dans le passé, dans une situation donnée, vous seriez 40% en faveur de l'écriture d'une fonction avec un booléen et 60%, et vous finirez par ne pas le faire. Peut-être qu'avec un bon soutien de l'IDE, le solde est maintenant de 55% l'écrivant comme ça et de 45% non. Et oui, vous aurez peut-être encore besoin de le lire en dehors de l'IDE, donc ce n'est pas parfait. Mais cela reste un compromis valable contre une alternative, comme ajouter une autre méthode ou une énumération pour clarifier le code autrement.
Sebastiaan van den Broek
6

2 jours et aucun n'a mentionné le polymorphisme?

target.UpdateRow(item);

Lorsque je suis un client souhaitant mettre à jour une ligne avec un élément, je ne veux pas penser au nom de la base de données, à l'URL du micro-service, au protocole utilisé pour communiquer ou au fait qu'un appel externe soit ou non devra être utilisé pour y arriver. Arrête de pousser ces détails sur moi. Prenez juste mon article et allez déjà mettre à jour une ligne quelque part.

Faites cela et cela devient une partie du problème de construction. Cela peut être résolu de plusieurs façons. En voici un:

Target xyzTargetFactory(TargetBuilder targetBuilder) {
    return targetBuilder
        .connectionString("some connection string")
        .table("table_name")
        .external()
        .build()
    ; 
}

Si vous regardez cela et que vous pensez "Mais mon langage a des arguments nommés, je n'ai pas besoin de cela". Bien, super, allez les utiliser. Gardez cette absurdité à l’abri du client appelant qui ne devrait même pas savoir à quoi il parle.

Il convient de noter que ceci est plus qu'un problème sémantique. C'est aussi un problème de design. Passez un booléen et vous êtes obligé d'utiliser des branches pour le gérer. Pas très orienté objet. Avez-vous deux booléens ou plus à passer? Souhaitant que votre langue ait plusieurs envois? Examinez ce que les collections peuvent faire lorsque vous les imbriquez. La bonne structure de données peut vous faciliter la vie.

confits_orange
la source
2

Si vous pouvez modifier la updateRowfonction, vous pouvez peut-être la reformuler en deux fonctions. Étant donné que la fonction prend un paramètre booléen, je suppose que cela ressemble à ceci:

function updateRow(var item, bool externalCall) {
  Database.update(item);

  if (externalCall) {
    Service.call();
  }
}

Cela peut être un peu une odeur de code. La fonction peut avoir un comportement radicalement différent en fonction de la externalCallvariable, auquel cas elle a deux responsabilités différentes. Le refactoriser en deux fonctions qui n'ont qu'une seule responsabilité peut améliorer la lisibilité:

function updateRowAndService(var item) {
  updateRow(item);
  Service.call();
}

function updateRow(var item) {
  Database.update(item);
}

Désormais, partout où vous appelez ces fonctions, vous pouvez savoir en un coup d’œil si le service externe est appelé.

Ce n'est pas toujours le cas, bien sûr. C'est la situation et une question de goût. Refactoriser une fonction qui prend un paramètre booléen en deux fonctions est généralement intéressant, mais ce n’est pas toujours le meilleur choix.

Kevin
la source
0

Si UpdateRow est dans une base de code contrôlée, je considérerais un modèle de stratégie:

public delegate void Request(string query);    
public void UpdateRow(Item item, Request request);

Où Request représente une sorte de DAO (un rappel dans un cas trivial).

Vrai cas:

UpdateRow(item, query =>  queryDatabase(query) ); // perform remote call

Faux cas:

UpdateRow(item, query => readLocalCache(query) ); // skip remote call

Habituellement, l'implémentation du point de terminaison est configurée à un niveau d'abstraction plus élevé et je l'utilise simplement ici. En tant qu'effet secondaire utile, cela me donne la possibilité d'implémenter un autre moyen d'accéder aux données distantes, sans aucune modification du code:

UpdateRow(item, query => {
  var data = readLocalCache(query);
  if (data == null) {
    data = queryDatabase(query);
  }
  return data;
} );

En général, une telle inversion de contrôle garantit un couplage moindre entre votre stockage de données et votre modèle.

Basilevs
la source