Selon Est-il erroné d’utiliser un paramètre booléen pour déterminer le comportement? , Je connais l’importance d’éviter d’utiliser des paramètres booléens pour déterminer un comportement, par exemple:
version originale
public void setState(boolean flag){
if(flag){
a();
}else{
b();
}
c();
}
nouvelle version:
public void setStateTrue(){
a();
c();
}
public void setStateFalse(){
b();
c();
}
Mais qu'en est-il du cas où le paramètre booléen est utilisé pour déterminer des valeurs plutôt que des comportements? par exemple:
public void setHint(boolean isHintOn){
this.layer1.visible=isHintOn;
this.layer2.visible=!isHintOn;
this.layer3.visible=isHintOn;
}
J'essaie d'éliminer le drapeau isHintOn et de créer 2 fonctions distinctes:
public void setHintOn(){
this.layer1.visible=true;
this.layer2.visible=false;
this.layer3.visible=true;
}
public void setHintOff(){
this.layer1.visible=false;
this.layer2.visible=true;
this.layer3.visible=false;
}
mais la version modifiée semble moins maintenable car:
il a plus de codes que la version originale
il ne peut pas clairement montrer que la visibilité de la couche 2 est opposée à l'option de conseil
quand un nouveau calque (par exemple: layer4) est ajouté, je dois ajouter
this.layer4.visible=false;
et
this.layer4.visible=true;
dans setHintOn () et setHintOff () séparément
Ma question est donc la suivante: si le paramètre booléen est utilisé pour déterminer des valeurs uniquement, mais pas des comportements (par exemple: no if-else sur ce paramètre), est-il toujours recommandé d’éliminer ce paramètre booléen?
setHint(boolean isHintOn)
tant que méthode privée et ajoutez publicsetHintOn
et lessetHintOff
méthodes qui appellent respectivementsetHint(true)
etsetHint(false)
.setHint(true|false)
. Pomme de terre pomme de terre. Au moins utiliser quelque chose commesetHint
etunsetHint
.is
au début.isValid
etc. Alors pourquoi changer cela pour deux mots? En outre, "plus naturel" est dans l'oeil du spectateur. Si vous voulez le prononcer comme une phrase anglaise, alors il serait plus naturel pour moi d'avoir "si l'indication est allumée" avec "le" caché dedans.Réponses:
La conception de l'API doit se concentrer sur ce qui est le plus utilisable pour un client de l'API, du côté appelant .
Par exemple, si cette nouvelle API nécessite que l'appelant écrive régulièrement du code comme celui-ci
alors il devrait être évident qu'éviter le paramètre est pire que d'avoir une API qui permet à l'appelant d'écrire
L'ancienne version ne produit qu'un problème qui doit ensuite être résolu (et probablement plus d'une fois) du côté de l'appelant. Cela n'améliore ni la lisibilité ni la maintenabilité.
L' implémentation , cependant, ne devrait pas dicter l'apparence de l'API publique. Si une fonction comme
setHint
avec un paramètre nécessite moins de code dans la mise en œuvre, mais une API en termes desetHintOn
/setHintOff
semble plus facile à utiliser pour un client, vous pouvez l'implémenter de cette façon:Ainsi, bien que l'API publique n'ait pas de paramètre booléen, il n'y a pas de logique de duplication ici, donc un seul endroit pour changer quand une nouvelle exigence (comme dans l'exemple de la question) arrive.
Cela fonctionne aussi dans le sens inverse: si le
setState
méthode ci-dessus doit basculer entre deux morceaux de code différents, ces morceaux de code peuvent être refactorisés en deux méthodes privées différentes. Donc, à mon humble avis, cela n’a aucun sens de rechercher un critère permettant de choisir entre "un paramètre / une méthode" et "zéro paramètre / deux méthodes" en examinant les éléments internes. Cependant, regardez comment vous voudriez que l’API joue le rôle de consommateur.En cas de doute, essayez d'utiliser "test driven development" (TDD), cela vous obligera à réfléchir à l'API publique et à son utilisation.
la source
SetLoanFacility(bool enabled)
, car, après avoir fourni un prêt, il peut s'avérer difficile de le supprimer, et les deux options peuvent impliquer une logique complètement différente - et vous voudriez alors séparer Créer / Supprimer les méthodes.Toggle()
la fonction à utiliser n'est pas correcte. C'est le point entier; si l'appelant ne se soucie que de "le changer" et non de "ce qui finit par", alorsToggle()
c'est l'option qui évite les vérifications et la prise de décision supplémentaires. Je n'appellerais pas cela un cas COMMUN, ni ne recommanderais de le rendre disponible sans raison valable, mais si l'utilisateur a besoin d'une bascule, je lui en donnerai une.Martin Fowler cite Kent Beck en recommandant des
setOn()
setOff()
méthodes séparées , mais dit également que cela ne devrait pas être considéré comme inviolable:Une autre recommandation consiste à utiliser une valeur énumérée ou un type d’indicateur pour donner,
true
etfalse
mieux, des noms spécifiques au contexte. Dans votre exemple,showHint
ethideHint
pourrait être mieux.la source
-[NSView setNeedsDisplay:]
méthode dans laquelle vous passezYES
si une vue doit être redessinée etNO
si elle ne devrait pas. Vous n'avez presque jamais besoin de le dire, donc UIKit n'a-[UIView setNeedsDisplay]
aucun paramètre. Il n'a pas la-setDoesNotNeedDisplay
méthode correspondante .bool isPremium
indicateur dans son exemple, mais j'utiliserais un enum (BookingType bookingType
) pour paramétrer la même méthode, à moins que la logique de chaque réservation ne soit assez différente. La "logique enchevêtrée" à laquelle fait référence Fowler est souvent souhaitable si on veut pouvoir voir quelle est la différence entre les deux modes. Et si elles sont radicalement différentes, j'exposerais la méthode paramétrée de manière externe et implémenter les méthodes séparées en interne.Je pense que vous mélangez deux choses dans votre message, l’API et la mise en œuvre. Dans les deux cas, je ne pense pas qu'il y ait une règle forte que vous pouvez utiliser tout le temps, mais vous devriez considérer ces deux choses indépendamment (autant que possible).
Commençons par l'API, à la fois:
et:
sont des alternatives valides selon ce que votre objet est censé offrir et comment vos clients vont utiliser l'API. Comme Doc l'a souligné, si vos utilisateurs ont déjà une variable booléenne (d'un contrôle d'interface utilisateur, d'une action utilisateur, d'une API externe, etc.), la première option est plus logique, sinon vous forcez simplement une instruction if supplémentaire sur le code du client. . Toutefois, si, par exemple, vous changez l’indice en true lors du début d’un processus et en false à la fin, la première option vous donne un résultat semblable à celui-ci:
tandis que la deuxième option vous donne ceci:
lequel OMI est beaucoup plus lisible, je vais donc utiliser la deuxième option dans ce cas. Évidemment, rien ne vous empêche d’offrir les deux options (ou plus, vous pouvez utiliser une énumération comme l’a dit Graham si cela semble plus logique, par exemple).
Le fait est que vous devez choisir votre API en fonction de ce que l'objet est censé faire et de la manière dont les clients vont l'utiliser, et non en fonction de la manière dont vous allez le mettre en œuvre.
Ensuite, vous devez choisir comment vous allez implémenter votre API publique. Supposons que nous ayons choisi les méthodes
setHintOn
etsetHintOff
que nous soyons notre API publique et qu'ils partagent cette logique commune, comme dans votre exemple. Vous pouvez facilement résumer cette logique par une méthode privée (code copié à partir de Doc):Inversement, supposons que nous ayons choisi
setHint(boolean isHintOn)
notre API, mais inversons votre exemple. Quelle que soit la raison, définir l'indice est complètement différent de le définir sur Désactivé. Dans ce cas, nous pourrions l'implémenter comme suit:Ou même:
Le fait est que, dans les deux cas, nous avons d'abord choisi notre API publique, puis adapté notre implémentation pour l'adapter à l'API choisie (et aux contraintes que nous avons), et non l'inverse.
Soit dit en passant, je pense que même pour le poste que vous avez lié sur l' utilisation d' un paramètre booléen pour déterminer le comportement, à savoir que vous devez décider en fonction de votre cas d'utilisation particulière plutôt que d' une règle stricte (bien que dans ce cas particulier généralement la bonne chose à faire est le casser dans plusieurs fonctions).
la source
begin
etend
ou synonymes, juste à préciser explicitement c'est ce qu'ils font, et impliquer qu'une le début doit avoir une fin et vice versa.using
blocs en C #,with
gestionnaires de contexte d’instruction en Python, en passant le corps de comme un objet lambda ou appelable (par exemple, la syntaxe de bloc Ruby), etc.Tout d’abord: le code n’est pas automatiquement moins facile à gérer, simplement parce qu’il est un peu plus long. La clarté est ce qui compte.
Maintenant, si vous êtes vraiment juste traiter les données, alors ce que vous avez est un setter pour une propriété booléenne. Dans ce cas, vous voudrez peut-être simplement stocker cette valeur directement et en déduire la visibilité des calques, c'est-à-dire
(J'ai pris la liberté de donner aux couches les noms réels - si vous ne l'avez pas dans votre code d'origine, je commencerais par ça)
Cela vous laisse toujours avec la question de savoir si vous avez une
setHintVisibility(bool)
méthode. Personnellement, je recommanderais de le remplacer par unshowHint()
et unehideHint()
méthode - les deux seront très simples et vous n’aurez pas à les changer lorsque vous ajoutez des calques. Ce n'est pas une coupe nette bien / mal, cependant.Maintenant, si l'appel de la fonction change réellement la visibilité de ces couches, vous avez en fait un comportement. Dans ce cas, je recommanderais certainement des méthodes séparées.
la source
showHint
vssetHintVisibility
). J'ai mentionné la nouvelle couche simplement parce que OP était inquiet à ce sujet. , Nous avons juste besoin aussi d'ajouter une nouvelle méthode:isLayer4Visible
.showHint
ethideHint
juste définir l'isHintVisible
attribut à vrai / faux et cela ne change pas.Les paramètres booléens sont bien dans le deuxième exemple. Comme vous l'avez déjà compris, les paramètres booléens ne sont pas problématiques en eux-mêmes. C'est changer de comportement en fonction d'un drapeau, ce qui est problématique.
Le premier exemple est toutefois problématique, car la dénomination indique une méthode de définition, mais les implémentations semblent être quelque chose de différent. Il existe donc un antipattern de changement de comportement et une méthode nommée de manière trompeuse. Mais si la méthode est en réalité une méthode habituelle (sans changement de comportement), alors il n'y a pas de problème avec
setState(boolean)
. Avoir deux méthodes,setStateTrue()
etsetStateFalse()
c'est compliquer inutilement les choses sans aucun avantage.la source
Une autre façon de résoudre ce problème consiste à introduire un objet représentant chaque indication et à lui confier la responsabilité de déterminer les valeurs booléennes associées à cette indication. De cette façon, vous pouvez ajouter de nouvelles permutations plutôt que de simplement avoir deux états booléens.
Par exemple, en Java, vous pourriez faire:
Et puis votre code d'appelant ressemblerait à ceci:
Et votre code d'implémentation ressemblerait à ceci:
Ceci permet de garder le code d'implémentation et le code de l'appelant concis, en échange de la définition d'un nouveau type de données qui mappe clairement les intentions nommées fortement typées aux ensembles d'états correspondants. Je pense que c'est mieux tout autour.
la source
Quand j'ai des doutes sur de telles choses. J'aime imaginer à quoi ressemblerait la trace de pile.
Pendant de nombreuses années, j'ai travaillé sur un projet PHP qui utilisait les mêmes fonctions qu'un setter et un getter . Si vous avez passé la valeur null , la valeur serait renvoyée. Sinon, définissez-la. C'était horrible de travailler avec .
Voici un exemple de trace de pile:
Vous n'aviez aucune idée de l' état interne et cela rendait le débogage plus difficile. Il existe une foule d'autres effets secondaires négatifs, mais essayez de voir qu'il existe une valeur dans un nom de fonction détaillé et une clarté lorsque les fonctions effectuent une seule action.
Imaginez maintenant que vous utilisez la trace de pile pour une fonction ayant un comportement A ou B basé sur une valeur booléenne.
C'est déroutant si vous me demandez. Les mêmes
setVisible
lignes donnent deux chemins de trace différents.Revenons donc à votre question. Essayez d'imaginer à quoi ressemblera la trace de pile, comment elle communique à une personne et demandez-vous si vous aidez une autre personne à déboguer le code.
Voici quelques conseils:
Parfois, le code semble excessif au microscope, mais lorsque vous revenez à l’ensemble, une approche minimaliste le fait disparaître. Si vous en avez besoin pour vous démarquer afin de pouvoir le maintenir. L'ajout de nombreuses petites fonctions peut sembler trop prolixe, mais cela améliore la facilité de maintenance lorsqu'il est utilisé largement dans un contexte plus large.
la source
setVisible
dérange dans la trace de pile, c’est que l’appelsetVisible(true)
semble donner lieu à un appel àsetVisible(false)
(ou l’inverse, en fonction de la façon dont vous avez obtenu cette trace).Dans presque tous les cas où vous passez un
boolean
paramètre à une méthode comme un indicateur pour modifier le comportement de quelque chose, vous devriez envisager une approche plus explicite façon de le faire et ce typesafe.Si vous ne faites rien d'autre que d'utiliser un
Enum
qui représente l'état, vous avez amélioré la compréhension de votre code.Cet exemple utilise la
Node
classe deJavaFX
:est toujours meilleur que, comme sur de nombreux
JavaFX
objets:mais je pense
.setVisible()
et suis.setHidden()
la meilleure solution pour la situation où le drapeau est unboolean
comme c'est le plus explicite et le moins verbeux.dans le cas de sélections multiples, il est encore plus important de le faire de cette façon.
EnumSet
existe juste pour cette raison.la source
Lorsque vous choisissez une approche pour une interface transmettant un paramètre (booléen) à une méthode surchargée sans ce paramètre, consultez les clients consommateurs.
Si toutes les utilisations transmettent des valeurs constantes (par exemple, true, false), cela plaide en faveur des surcharges.
Si toutes les utilisations passaient avec une valeur de variable, cela plaiderait pour la méthode avec approche paramétrique.
Si aucun de ces extrêmes ne s'applique, cela signifie qu'il existe une combinaison d'usages de clients. Vous devez donc choisir de prendre en charge les deux formulaires ou de faire en sorte qu'une catégorie de clients s'adapte à l'autre (ce qui pour eux est un style plus artificiel).
la source
Il y a deux considérations à concevoir:
Ils ne devraient pas être confondus.
C'est parfaitement bien pour:
En tant que tel, tout argument qui tente d'équilibrer les coûts / avantages d'une conception d'API par les coûts / avantages d'une conception de mise en œuvre est douteux et doit être examiné avec soin.
Côté API
En tant que programmeur, je privilégierai généralement les API programmables. Le code est beaucoup plus clair quand je peux transmettre une valeur que quand j'ai besoin d'un
if
/switch
énoncé de la valeur pour décider de la fonction à appeler.Ce dernier peut être nécessaire si chaque fonction attend des arguments différents.
Dans votre cas, donc, une seule méthode
setState(type value)
semble donc préférable.Cependant , il n'y a rien que nameless pire
true
,false
,2
, etc ... ces valeurs magiques ont pas de sens de leur propre chef. Évitez l’obsession primitive et adoptez un typage fort.Ainsi, à partir d' un POV API, je veux:
setState(State state)
.Du côté de la mise en œuvre
Je recommande de faire ce qui est plus facile.
Si la méthode est simple, il vaut mieux la garder ensemble. Si le flux de contrôle est compliqué, il est préférable de le séparer en plusieurs méthodes, chacune traitant un sous-cas ou une étape du pipeline.
Enfin, envisagez de grouper .
Dans votre exemple (avec des espaces ajoutés pour la lisibilité):
Pourquoi
layer2
casser la tendance? Est-ce une fonctionnalité ou est-ce un bug?Pourrait-il être possible d’avoir 2 listes
[layer1, layer3]
et[layer2]
, avec un explicite nom indiquant la raison pour laquelle elles ont été regroupées, puis de les parcourir par la suite?Par exemple:
Le code parle de lui-même, il est clair pourquoi il existe deux groupes et leur comportement diffère.
la source
Séparément de la question
setOn() + setOff()
vsset(flag)
, je voudrais examiner avec soin si un type booléen est le meilleur ici. Êtes-vous sûr qu'il n'y aura jamais de troisième option?Cela peut valoir la peine d’envisager une énumération au lieu d’un booléen. En plus de permettre l’extensibilité, il est également difficile de placer le booléen dans le mauvais sens, par exemple:
contre
Avec l’énumération, il sera beaucoup plus facile d’agrandir si une personne décide qu’elle souhaite une option «si nécessaire»:
contre
la source
Je suggérerais de réévaluer cette connaissance.
Tout d'abord, je ne vois pas la conclusion que vous proposez dans la question SE que vous avez liée. Ils parlent principalement de transférer un paramètre sur plusieurs étapes d'appels de méthode, où il est évalué très loin dans la chaîne.
Dans votre exemple, vous évaluez le paramètre directement dans votre méthode. À cet égard, il ne diffère en rien de tout autre type de paramètre.
En général, utiliser des paramètres booléens n’est absolument pas un problème. et évidemment tout paramètre déterminera un comportement, ou pourquoi voudriez-vous l'avoir en premier lieu?
la source
Définir la question
Votre question de titre est "Est-ce que c'est mal de [...]?" - mais que voulez-vous dire par "faux"?
Selon un compilateur C # ou Java, ce n'est pas faux. Je suis certain que vous en êtes conscient et ce n'est pas ce que vous demandez. J'ai bien peur que cela ne soit différent de celui des
n
programmeursn+1
. Cette réponse présente ce que le livre Clean Code a à dire à ce sujet.Répondre
Clean Code présente des arguments solides contre les arguments de fonction en général:
Un "lecteur" peut ici être le consommateur d'API. Il peut également s'agir du prochain codeur, qui ne sait pas encore ce que fait ce code - ce qui pourrait être vous dans un mois. Ils passeront par 2 fonctions séparément ou par 1 fonction deux fois , une
true
foisfalse
dans l’esprit et une fois dans l’esprit.En bref, utilisez le moins d'arguments possible .
Le cas spécifique d'un argument de drapeau est ensuite adressé directement:
Afin de répondre directement à vos questions:
Selon Clean Code , il est recommandé d’éliminer ce paramètre.
Information additionnelle:
Votre exemple est assez simple, mais même là, vous pouvez voir la simplicité se propager à votre code: les fonctions sans paramètre ne font que des affectations simples, alors que l’autre fonction doit faire de l’arithmétique booléenne pour atteindre le même objectif. Dans cet exemple simplifié, l'arithmétique booléenne est triviale, mais pourrait être assez complexe dans une situation réelle.
J'ai vu beaucoup d'arguments ici pour rendre cela dépendant de l'utilisateur de l'API, car il serait stupide de le faire dans beaucoup d'endroits:
Je fais suis d' accord que quelque chose de non-optimal se passe ici. C’est peut-être trop évident à voir, mais votre toute première phrase mentionne "l’importance d’éviter d’utiliser des paramètres booléens pour déterminer un comportement" et c’est la base de toute la question. Je ne vois pas pourquoi il serait difficile de faciliter cette tâche pour l'utilisateur de l'API.
Je ne sais pas si vous testez - dans ce cas, considérez également ceci:
la source