Est-ce une erreur d'utiliser un paramètre booléen pour déterminer les valeurs?

39

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:

  1. il a plus de codes que la version originale

  2. il ne peut pas clairement montrer que la visibilité de la couche 2 est opposée à l'option de conseil

  3. 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?

ocomfd
la source
26
Ce n'est jamais mauvais si le code résultant est plus lisible et maintenable ;-) Je recommanderais d'utiliser la méthode unique au lieu des deux méthodes séparées.
Helb
32
Vous présentez un argument convaincant selon lequel une seule implémentation d'une méthode qui définit ces booléens entraînera une maintenance plus facile de la classe et une meilleure compréhension de son implémentation. Très bien; ce sont des considérations légitimes. Mais l' interface publique de la classe n'a pas besoin d'être déformée pour les accueillir. Si des méthodes distinctes facilitent la compréhension et l’utilisation de l’interface publique, définissez-la en setHint(boolean isHintOn)tant que méthode privée et ajoutez public setHintOnet les setHintOffméthodes qui appellent respectivement setHint(true)et setHint(false).
Mark Amery
9
Je serais très mécontent de ces noms de méthodes: ils n'offrent vraiment aucun avantage setHint(true|false). Pomme de terre pomme de terre. Au moins utiliser quelque chose comme setHintet unsetHint.
Konrad Rudolph
4
@kevincline Si la condition est un nom, vous écrivez isau début. isValidetc. 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.
M. Lister

Réponses:

95

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

if(flag)
    foo.setStateTrue();
else
    foo.setStateFalse();

alors il devrait être évident qu'éviter le paramètre est pire que d'avoir une API qui permet à l'appelant d'écrire

 foo.setState(flag);

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 setHintavec un paramètre nécessite moins de code dans la mise en œuvre, mais une API en termes desetHintOn / setHintOffsemble plus facile à utiliser pour un client, vous pouvez l'implémenter de cette façon:

private void setHint(boolean isHintOn){
    this.layer1.visible=isHintOn;
    this.layer2.visible=!isHintOn;
    this.layer3.visible=isHintOn;
}

public void setHintOn(){
   setHint(true);
}

public void setHintOff(){
   setHint(false);
}

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.

Doc Brown
la source
DocBrown, diriez-vous que la ligne de démarcation appropriée consiste à déterminer si le réglage de chaque état a des effets secondaires complexes et éventuellement non réversibles? Par exemple, si vous faites simplement basculer un indicateur qui fait ce qui est écrit, et qu'il n'y a pas de machine à états sous-jacente, vous paramétrez les différents états. Alors que, par exemple, vous ne paramètreriez pas une méthode comme 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.
Steve
15
@Steve: vous essayez toujours de concevoir l'API à partir des exigences que vous voyez du côté de l'implémentation. Pour être franc: c'est totalement hors de propos. Utilisez la variante de l'API publique la plus facile à utiliser du côté appelant. En interne, vous pouvez toujours laisser deux méthodes publiques appeler une privée avec un paramètre. Ou inversement, vous pouvez laisser une méthode avec un paramètre basculer entre deux méthodes privées avec une logique différente.
Doc Brown
@Steve: voir mon édition.
Doc Brown
Je prends tous vos arguments - j'y réfléchis en fait du côté de l'appelant (d'où ma référence à "ce qui est écrit sur l'étain") et j'essaie de formuler la règle appropriée selon laquelle un appelant s'attend habituellement à utiliser chaque approche. Il me semble que la règle est de savoir si l'appelant s'attend à ce que les appels répétés soient idempotents et aux transitions d'état non contraignantes et sans effets secondaires complexes. Activer et désactiver un commutateur d'éclairage de pièce serait paramétré, activer et désactiver une centrale régionale serait multi-méthode.
Steve
1
@Steve Donc, si l'utilisateur a besoin d'affirmer un paramètre spécifique, 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", alors Toggle()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.
Kamil Drakari
40

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:

Si vous extrayez des données à partir d'une source booléenne, telle qu'un contrôle d'interface utilisateur ou une source de données, je préférerais setSwitch(aValue)que

if (aValue)
  setOn();
else
  setOff();

Ceci est un exemple dans lequel une API doit être écrite pour faciliter la tâche de l'appelant. Par conséquent, si nous savons d'où vient l'appelant, nous devons concevoir l'API en tenant compte de ces informations. Cela indique également que nous pouvons parfois fournir les deux styles si nous recevons des appelants dans les deux sens.

Une autre recommandation consiste à utiliser une valeur énumérée ou un type d’indicateur pour donner, trueet falsemieux, des noms spécifiques au contexte. Dans votre exemple, showHintet hideHintpourrait être mieux.

Graham Lee
la source
16
D'après mon expérience, setSwitch (valeur) donne presque toujours moins de code que setOn / setOff, précisément à cause du code if / else cité dans votre réponse. J'ai tendance à maudire les développeurs qui me donnent une API de setOn / setOff plutôt que de setSwitch (valeur).
17 du 26
1
Envisagez-le d'un point de vue similaire: si vous devez coder en dur la valeur, c'est facile avec l'un ou l'autre. Toutefois, si vous devez le définir sur, par exemple, une entrée utilisateur, si vous pouvez simplement transmettre la valeur directement, cela enregistre une étape.
La poursuite de Monica le
@ 17 sur 26 comme contre-exemple concret (pour montrer que "ça dépend" plutôt que celui qui est préférable à l'autre), dans AppKit d'Apple, il existe une -[NSView setNeedsDisplay:]méthode dans laquelle vous passez YESsi une vue doit être redessinée et NOsi 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 -setDoesNotNeedDisplayméthode correspondante .
Graham Lee
2
@GrahamLee, je pense que l'argument de Fowler est assez subtil et repose vraiment sur le jugement. Je n'utiliserais pas d' bool isPremiumindicateur 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.
Steve
3

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:

public void setHint(boolean isHintOn)

et:

public void setHintOn()
public void setHintOff()

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:

setHint(true)
// Do your process
…
setHint(false)

tandis que la deuxième option vous donne ceci:

setHintOn()
// Do your process
…
setHintOff()

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 setHintOnet setHintOffque 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):

private void setHint(boolean isHintOn){
    this.layer1.visible=isHintOn;
    this.layer2.visible=!isHintOn;
    this.layer3.visible=isHintOn;
}

public void setHintOn(){
   setHint(true);
}

public void setHintOff(){
   setHint(false);
}

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:

public void setHint(boolean isHintOn){
    if(isHintOn){
        // Set it On
    } else {
        // Set it Off
    }    
}

Ou même:

public void setHint(boolean isHintOn){    
    if(isHintOn){
        setHintOn()
    } else {
        setHintOff()
   }    
}

private void setHintOn(){
   // Set it On
}

private void setHintOff(){
   // Set it Off 
}

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).

jesm00
la source
3
Comme une note de côté, si elle est quelque chose comme le pseudo-bloc (une instruction au début, une à la fin), vous devriez probablement utiliser beginet endou 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.
Fonds de la poursuite de Monica le
Je suis d’accord avec Nic et ajoute: si vous voulez vous assurer que début et fin sont toujours couplés, vous devez également fournir des idiomes spécifiques à la langue: gabarits RAII / scope en C ++, usingblocs en C #, withgestionnaires 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.
Daniel Pryden
Je suis d'accord avec les deux points, j'essayais simplement de donner un exemple simple pour illustrer l'autre cas (bien que ce soit un mauvais exemple, comme vous l'avez tous deux souligné :)).
jesm00
2

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

bool isBackgroundVisible() {
    return isHintVisible;
}    

bool isContentVisible() {
    return !isHintVisible;
}

(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 un showHint()et une hideHint()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.

vous double
la source
So tl; dr est: Vous recommandez de scinder en deux fonctions car vous n’aurez pas à les changer lorsque vous ajoutez des calques? Si une couche 4 est ajoutée, nous devrons peut-être également indiquer "this.layer4.visibility = isHintOn", donc je ne suis pas sûr d'être d'accord. Au contraire, c'est un inconvénient, car maintenant, lorsqu'un calque est ajouté, nous devons éditer deux fonctions, pas une seule.
Erdrik Ironrose
Non, je le recommande (faiblement) pour plus de clarté ( showHintvs setHintVisibility). 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. showHintet hideHintjuste définir l' isHintVisibleattribut à vrai / faux et cela ne change pas.
doubleVous
1
@doubleYou, vous dites que le code plus long n'est pas automatiquement moins facile à gérer. Je dirais que la longueur du code est l’une des principales variables de maintenabilité et de clarté, dépassée seulement par la complexité structurelle. Tout code qui devient plus long et plus complexe structurellement doit le mériter, sinon les problèmes simples reçoivent un traitement plus complexe qu’ils ne le méritent, et le code-base acquiert des bosquets de lignes inutiles (le problème de «trop de niveaux d’indirection»).
Steve
@Steve Je suis tout à fait d'accord pour dire que l'on peut trop concevoir, c'est-à-dire rallonger le code sans le rendre plus clair - otoh, on peut toujours raccourcir le code au prix de la clarté. Il n'y a donc pas de relation 1: 1 ici.
doubleVous
@Steve Pensez «code golf» - prendre ce code et le réécrire en plusieurs lignes le rend plus clair. "Code golf" est une extrême, mais il y a encore beaucoup de programmeurs qui pensent que tout mettre dans une expression intelligente est "élégant" et peut-être même plus vite parce que les compilateurs ne optimisent pas assez bien.
BlackJack
1

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()et setStateFalse()c'est compliquer inutilement les choses sans aucun avantage.

JacquesB
la source
1

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:

public enum HintState {
    SHOW_HINT(true, false, true),
    HIDE_HINT(false, true, false);

    private HintState(boolean layer1Visible, boolean layer2Visible, boolean layer3Visible) {
         // constructor body and accessors omitted for clarity
    }
}

Et puis votre code d'appelant ressemblerait à ceci:

setHint(HintState.SHOW_HINT);

Et votre code d'implémentation ressemblerait à ceci:

public void setHint(HintState hint) {
    this.layer1Visible = hint.isLayer1Visible();
    this.layer2Visible = hint.isLayer2Visible();
    this.layer3Visible = hint.isLayer3Visible();
}

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.

Daniel Pryden
la source
0

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?

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:

function visible() : line 440
function parent() : line 398
function mode() : line 384
function run() : line 5

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.

function bar() : line 92
function setVisible() : line 120
function foo() : line 492
function setVisible() : line 120
function run() : line 5

C'est déroutant si vous me demandez. Les mêmes setVisiblelignes 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:

  • un nom de fonction clair qui implique une intention sans avoir besoin de connaître les valeurs d'argument.
  • une fonction effectue une seule action
  • le nom implique une mutation ou un comportement immuable
  • résoudre les problèmes de débogage relatifs à la capacité de débogage du langage et des outils.

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.

Réactionnel
la source
1
Ce qui me setVisibledérange dans la trace de pile, c’est que l’appel setVisible(true)semble donner lieu à un appel à setVisible(false)(ou l’inverse, en fonction de la façon dont vous avez obtenu cette trace).
David K
0

Dans presque tous les cas où vous passez un booleanparamè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 Enumqui représente l'état, vous avez amélioré la compréhension de votre code.

Cet exemple utilise la Nodeclasse de JavaFX:

public enum Visiblity
{
    SHOW, HIDE

    public boolean toggleVisibility(@Nonnull final Node node) {
        node.setVisible(!node.isVisible());
    }
}

est toujours meilleur que, comme sur de nombreux JavaFXobjets:

public void setVisiblity(final boolean flag);

mais je pense .setVisible()et suis .setHidden()la meilleure solution pour la situation où le drapeau est un booleancomme 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. EnumSetexiste juste pour cette raison.

Ed Mann a un très bon article sur ce sujet. J'étais sur le point de paraphraser ce qu'il a dit, pour ne pas faire double emploi, je vais simplement poster un lien vers son blog en guise d'addendum à cette réponse.


la source
0

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).

Erik Eidt
la source
Que se passe-t-il si vous concevez un système intégré et que vous êtes à la fois le producteur de code et le "client consommateur" du code? Comment une personne à la place d'un client consommateur devrait-elle exprimer sa préférence pour une approche par rapport à une autre?
Steve
@ Step, en tant que client consommateur, vous savez si vous passez une constante ou une variable. Si vous transmettez des constantes, préférez les surcharges sans le paramètre.
Erik Eidt
Mais je voudrais expliquer pourquoi cela devrait être le cas. Pourquoi ne pas utiliser des énumérations pour un nombre limité de constantes, puisqu'il s'agit d'une syntaxe légère dans la plupart des langues, conçue précisément pour ce type de tâche?
Steve
@Steve, si nous savons au moment de la conception / compilation que les clients utiliseraient une valeur constante (vrai / faux) dans tous les cas, cela suggère qu'il existe en réalité deux méthodes spécifiques différentes plutôt qu'une méthode généralisée (qui prend un paramètre). Je dirais contre l’introduction de la généralité d’une méthode paramétrée quand elle n’est pas utilisée - c’est un argument de YAGNI.
Erik Eidt
0

Il y a deux considérations à concevoir:

  • API: quelle interface vous présentez à l'utilisateur,
  • Mise en oeuvre: clarté, maintenabilité, etc ...

Ils ne devraient pas être confondus.

C'est parfaitement bien pour:

  • avoir plusieurs méthodes dans le délégué API pour une seule implémentation,
  • avoir une seule méthode dans la répartition d'API pour plusieurs implémentations en fonction de la condition.

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é):

this.layer1.visible = isHintOn;
this.layer2.visible = ! isHintOn;
this.layer3.visible = isHintOn;

Pourquoi layer2casser 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:

for (auto layer : this.mainLayers) { // layer2
    layer.visible = ! isHintOn;
}
for (auto layer : this.hintLayers) { // layer1 and layer3
    layer.visible = isHintOn;
}

Le code parle de lui-même, il est clair pourquoi il existe deux groupes et leur comportement diffère.

Matthieu M.
la source
0

Séparément de la question setOn() + setOff()vs set(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:

setHint(false)

contre

setHint(Visibility::HIDE)

Avec l’énumération, il sera beaucoup plus facile d’agrandir si une personne décide qu’elle souhaite une option «si nécessaire»:

enum class Visibility {
  SHOW,
  HIDE,
  IF_NEEDED // New
}

contre

setHint(false)
setHint(true)
setHintAutomaticMode(true) // New
Phil
la source
0

Selon ..., je connais l'importance d'éviter d'utiliser des paramètres booléens pour déterminer un comportement

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?

AnoE
la source
0

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 nprogrammeurs n+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:

Les arguments sont difficiles. Ils prennent beaucoup de pouvoir conceptuel. [...] nos lecteurs auraient dû l'interpréter à chaque fois.

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 truefois falsedans 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:

Les arguments d'indicateur sont laids. Passer un booléen à une fonction est une pratique vraiment terrible. Cela complique immédiatement la signature de la méthode, proclamant à haute voix que cette fonction fait plus d’une chose. Cela fait une chose si le drapeau est vrai et une autre si le drapeau est faux!

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:

if (isAfterSunset) light.TurnOn();
else light.TurnOff();

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:

Les arguments sont encore plus difficiles du point de vue des tests. Imaginez la difficulté d'écrire tous les cas de test pour vous assurer que toutes les différentes combinaisons d'arguments fonctionnent correctement. S'il n'y a pas d'arguments, c'est trivial.

R. Schmitz
la source
Vous avez vraiment enterré la phrase ici: "... 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 là la base de toute la question. Je ne vois pas. une raison pour rendre cette chose qui est mauvaise à faire plus facile pour l'utilisateur de l'API. " C'est un point intéressant, mais vous affaiblissez plutôt votre propre argument dans votre dernier paragraphe.
Wildcard
Enterré la lede? Le noyau de cette réponse est "utilisez le moins d'arguments possible", comme expliqué dans la première moitié de cette réponse. Toutes les informations qui suivent ne sont que des informations supplémentaires: réfuter un argument contradictoire (par un autre utilisateur, pas OP) et quelque chose qui ne s'applique pas à tout le monde.
R. Schmitz
Le dernier paragraphe tente simplement de préciser que la question du titre n’est pas assez bien définie pour pouvoir y répondre. OP demande si c'est "faux", mais ne dit pas selon qui ou quoi. Selon le compilateur? Ressemble à un code valide, donc pas mal. Selon le livre Clean Code? Il utilise un argument drapeau, alors oui, il est "faux". Cependant, j'écris plutôt "recommandé", parce que pragmatique> dogmatique. Pensez-vous que je dois clarifier cela?
R. Schmitz
alors attendez, votre défense de votre réponse est que la question du titre est trop peu claire pour qu'on y réponde? : D OK ... J'ai en fait pensé que le point que j'ai cité était une nouvelle prise intéressante.
Wildcard
1
Clairement clair maintenant; bon travail!
Wildcard