Définir une variable pour nommer un argument de méthode est-il une bonne pratique?

Par souci de lisibilité, je suis souvent amené à définir des variables temporaires lors de l'appel de fonctions, telles que le code suivant.

var preventUndo = true;
doSomething(preventUndo);

La version plus courte de ceci à ceci serait,

doSomething(true);

Mais quand je reviens au code, je me demande souvent à quoi on se trueréfère. Existe-t-il une convention pour ce genre d’énigme?

Explication des variables

Votre cas est un exemple d' introduction expliquant le refactoring des variables . En bref, une variable explicative est une variable qui n'est pas strictement nécessaire, mais permet de donner un nom clair à quelque chose, dans le but d'améliorer la lisibilité.

Un code de bonne qualité communique l’ intention du lecteur; et en tant que développeur professionnel, la lisibilité et la maintenabilité sont vos objectifs n ° 1.

En tant que telle, la règle empirique que je recommanderais est la suivante: si le but de votre paramètre n’est pas immédiatement évident, n'hésitez pas à utiliser une variable pour lui donner un bon nom. Je pense que c'est une bonne pratique en général (à moins d'abus). Voici un exemple rapide et artificiel - considérez:

editButton.Enabled = (_grid.SelectedRow != null && ((Person)_grid.SelectedRow).Status == PersonStatus.Active);

par rapport au légèrement plus long, mais peut-être plus clair:

bool personIsSelected = (_grid.SelectedRow != null);
bool selectedPersonIsEditable = (personIsSelected && ((Person)_grid.SelectedRow).Status == PersonStatus.Active)
editButton.Enabled = (personIsSelected && selectedPersonIsEditable);

Paramètres booléens

Votre exemple montre en réalité pourquoi les booléens dans les API sont souvent une mauvaise idée . Du côté des appels, ils ne font rien pour expliquer ce qui se passe. Considérer:

ParseFolder(true, false);

Vous devez rechercher la signification de ces paramètres. s'ils étaient des énums, ce serait beaucoup plus clair:

ParseFolder(ParseBehaviour.Recursive, CompatibilityOption.Strict);

Modifier:

Ajout de titres et échange de l'ordre des deux paragraphes principaux, car trop de personnes se concentraient sur la partie relative aux paramètres booléens (pour être juste, il s'agissait du premier paragraphe à l'origine). Également ajouté un exemple à la première partie.

N'écris pas le code dont tu n'as pas besoin.

Si vous avez du doSomething(true)mal à comprendre, vous devez soit ajouter un commentaire:

// Do something and prevent the undo.
doSomething(true);

ou, si la langue le permet, ajoutez le nom du paramètre:

doSomething(preventUndo: true);

Sinon, comptez sur votre IDE pour vous donner la signature de la méthode appelée:

Cas utiles

Mettre une variable supplémentaire peut être utile:

1. Pour le débogage:

   var productId = this.Data.GetLastProductId();
   this.Data.AddToCart(productId);
   

   Le même code peut être écrit sur une seule ligne, mais si vous voulez placer un point d'arrêt avant d'ajouter un produit au panier afin de voir si l'ID de produit est correct, écrire deux lignes au lieu d'une seule est une bonne idée.

2. Si vous avez une méthode avec beaucoup de paramètres et que chaque paramètre passe d’une évaluation. Dans une ligne, cela peut devenir totalement illisible.

   // Even with indentation, this is unreadable.
   var doSomething(
       isSomethingElse ? 0 : this.getAValue(),
       this.getAnotherOne() ?? this.default,
       (a + b + c + d + e) * f,
       this.hello ? this.world : (this.hello2 ? this.world2 : -1));
   

3. Si l'évaluation d'un paramètre est trop compliquée. Un exemple de code que j'ai vu:

   // Wouldn't it be easier to have several if/else's (maybe even in a separate method)?
   do(something ? (hello ? world : -1) : (programmers ? stackexchange : (com ? -1 : 0)));
   

Pourquoi pas dans d'autres cas?

Pourquoi ne devriez-vous pas créer de variables supplémentaires dans des cas simples?

1. Pas à cause de l'impact sur les performances. Ce serait une très mauvaise hypothèse de la part d'un développeur débutant qui optimise son application. Il n'y a pas d'impact sur les performances dans la plupart des langages, car le compilateur insère la variable en ligne. Dans les langages où le compilateur ne le fait pas, vous pouvez gagner quelques microsecondes en l’alignant manuellement, ce qui ne vaut pas la peine. Ne fais pas ça.

2. Mais à cause du risque de dissocier le nom que vous donnez à la variable au nom du paramètre.

   Exemple:

   Disons que le code original est:

   void doSomething(bool preventUndo)
   {
       // Does something very interesting.
       undoHistory.removeLast();
   }
   
   // Later in code:
   var preventUndo = true;
   doSomething(preventUndo);
   

   Plus tard, un développeur travaillant sur des doSomethingnotifications qui récemment, l’historique des annulations, a introduit deux nouvelles méthodes:

   undoHistory.clearAll() { ... }
   undoHistory.disable() { ... }
   

   Maintenant, preventUndosemble pas très clair. Cela signifie-t-il que seule la dernière action sera empêchée? Ou peut-être que cela signifie que l'utilisateur ne pourra plus utiliser la fonction d'annulation? Ou que l'historique d'annulation sera effacé? Les noms les plus clairs seraient:

   doSomething(bool hideLastUndo) { ... }
   doSomething(bool removeAllUndo) { ... }
   doSomething(bool disableUndoFeature) { ... }
   

   Alors maintenant vous avez:

   void doSomething(bool hideLastUndo)
   {
       // Does something very interesting.
       undoHistory.removeLast();
   }
   
   // Later in code, very error prone, while the signature of the method is clear:
   var preventUndo = true;
   doSomething(preventUndo);
   

Qu'en est-il des enums?

D'autres personnes ont suggéré d'utiliser des enums. Bien que cela résolve le problème immédiat, il en crée un plus gros. Nous allons jeter un coup d'oeil:

enum undoPrevention
{
    keepInHistory,
    prevent,
}

void doSomething(undoPrevention preventUndo)
{
    doTheJob();
    if (preventUndo == undoPrevention.prevent)
    {
        this.undoHistory.discardLastEntry();
    }
}

doSomething(undoPrevention.prevent);

Des problèmes avec ça:

1. C'est trop de code. if (preventUndo == undoPrevention.prevent)? Sérieusement?! Je ne veux pas écrire de tels ifs à chaque fois.

2. L'ajout d'un élément à l'énumération est très tentant plus tard, si j'utilise la même enum ailleurs. Et si je le modifie comme ceci:

   enum undoPrevention
   {
       keepInHistory,
       prevent,
       keepButDisable, // Keeps the entry in the history, but makes it disabled.
   }
   

   Ce qui va se passer maintenant? La doSomethingméthode fonctionnera-t-elle comme prévu? Pour éviter cela, il faudrait écrire cette méthode depuis le début:

   void doSomething(undoPrevention preventUndo)
   {
       if (![undoPrevention.keepInHistory, undoPrevention.prevent].contains(preventUndo))
       {
           throw new ArgumentException('preventUndo');
       }
   
       doTheJob();
       if (preventUndo == undoPrevention.prevent)
       {
           this.undoHistory.discardLastEntry();
       }
   }
   

   La variante qui utilise des booléens commence à être si belle!

   void doSomething(bool preventUndo)
   {
       doTheJob();
       if (preventUndo)
       {
           this.undoHistory.discardLastEntry();
       }
   }
   

Ni l'un ni l'autre, vous ne devriez pas écrire de méthodes avec des drapeaux booléens.

Selon Robert C. Martin, dans son livre Clean Code (ISBN-13 978-0-13-235088-4), "Transférer un booléen dans une fonction est une pratique vraiment terrible."

Pour le paraphraser, le raisonnement est que le fait que vous ayez un commutateur true / false signifie que votre méthode fait probablement deux choses différentes (c'est-à-dire "faire quelque chose avec undo" et "faire quelque chose sans undo"), et devrait donc être diviser en deux méthodes différentes (qui peuvent appeler la même chose en interne).

DoSomething()
{...

DoSomethingUndoable()
{....

Quand vous regardez deux classes pour voir comment Couplé ils sont, l' une des catégories est un couplage de données , qui fait référence au code dans une classe appelant des méthodes d' une autre classe et juste de passage des données, comme ce mois , vous voulez un rapport et un autre catégorie est le couplage de contrôle , l’appel de méthodes et la transmission de quelque chose qui contrôle le comportement de la méthode. L'exemple que j'utilise en classe est un verbosedrapeau ou un reportTypedrapeau, mais preventUndoc'est aussi un excellent exemple. Comme vous venez de le démontrer, le couplage de contrôle rend difficile la lecture du code d'appel et la compréhension de ce qui se passe. Cela rend également le code appelant vulnérable aux modifications doSomething()qui utilisent le même indicateur pour contrôler à la fois l'annulation et l'archivage, ou ajouter un autre paramètre àdoSomething() contrôler l'archivage, brisant ainsi votre code, etc.

Le problème est que le code est trop étroitement lié. Passer un coup pour contrôler le comportement est, à mon avis, le signe d'une mauvaise API. Si vous possédez l'API, changez-la. Deux méthodes, doSomething()et doSomethingWithUndo(), serait mieux. si vous ne le possédez pas, écrivez vous-même deux méthodes d'encapsulation dans le code que vous possédez et faites appel à l'une d'elles doSomething(true)et à l'autre doSomething(false).

L'appelant n'a pas pour rôle de définir le rôle des arguments. Je vais toujours pour la version en ligne, et vérifie la signature de la fonction appelée si j'ai un doute.

La variable doit être nommée d'après son rôle dans la portée actuelle. Non La portée où ils sont envoyés.

Ce que je ferais en JavaScript est que la fonction prenne un objet comme seul paramètre, comme ceci:

function doSomething(settings) {
    var preventUndo = settings.hasOwnProperty('preventUndo') ? settings.preventUndo : false;
    // Deal with preventUndo in the normal way.
}

Puis appelez-le avec:

doSomething({preventUndo: true});

J'aime utiliser les fonctionnalités linguistiques pour clarifier moi-même. Par exemple, en C #, vous pouvez spécifier les paramètres par leur nom:

 CallSomething(name: "So and so", age: 12, description: "A random person.");

En JavaScript, je préfère généralement utiliser un objet options comme argument pour cette raison:

function doSomething(args) { /*...*/ }

doSomething({ name: 'So and so', age: 12, description: 'A random person.' });

C'est juste ma préférence cependant. Je suppose que cela dépendra beaucoup de la méthode appelée et de la façon dont l'EDI aide le développeur à comprendre la signature.

Je trouve que c'est le plus simple et le plus facile à lire:

enum Undo { ALLOW, PREVENT }

doSomething(Undo u) {
    if (Undo.ALLOW == u) {
        // save stuff to undo later.
    }
    // do your thing
}

doSomething(Undo.ALLOW);

MainMa n'aime pas ça. Il se peut que nous devions accepter d'être en désaccord ou utiliser une solution proposée par Joshua Bloch:

enum Undo {
    ALLOW {
        @Override
        public void createUndoBuffer() {
            // put undo code here
        }
    },
    PREVENT {
        @Override
        public void createUndoBuffer() {
            // do nothing
        }
    };

    public abstract void createUndoBuffer();
}

doSomething(Undo u) {
    u.createUndoBuffer();
    // do your thing
}

Maintenant, si vous ajoutez un jour Undo.LIMITED_UNDO, votre code ne sera pas compilé à moins que vous n'implémentiez la méthode createUndoBuffer (). Et dans quelque chose () il n'y a pas de si (Undo.ALLOW == u). Je l'ai fait dans les deux sens et la deuxième méthode est assez lourde et un peu difficile à comprendre lorsque l'énumération d'annulation s'étend aux pages et aux pages de code, mais cela vous fait réfléchir. J'utilise généralement la méthode la plus simple pour remplacer un booléen simple par un enum de 2 valeurs jusqu'à ce que j'ai des raisons de changer. Quand j'ajoute une troisième valeur, j'utilise mon IDE pour "Rechercher-usages" et tout réparer.

Je pense que votre solution rend votre code un peu plus lisible, mais j’éviterais de définir une variable supplémentaire simplement pour rendre votre appel de fonction plus clair. En outre, si vous souhaitez utiliser une variable supplémentaire, je la marquerai comme constante si votre langage de programmation le prend en charge.

Je peux penser à deux alternatives qui n'impliquent pas de variable supplémentaire:

1. Utilisez un commentaire supplémentaire

doSomething(/* preventUndo */ true);

2. Utilisez un enum à deux valeurs au lieu de boolean

enum Options
{
    PreventUndo,
    ...
}

...

doSomething(PreventUndo);

Vous pouvez utiliser la variante 2 si votre langue prend en charge les énumérations.

MODIFIER

Bien sûr, utiliser des arguments nommés est également une option, si votre langue les prend en charge.

En ce qui concerne le codage supplémentaire requis par les enums, il me semble vraiment négligeable. Au lieu de

if (preventUndo)
{
    ...
}

tu as

if (undoOption == PreventUndo)
{
    ...
}

ou

switch (undoOption)
{
  case PreventUndo:
  ...
}

Et même si la saisie est un peu plus fréquente, rappelez-vous que le code est écrit une fois et lu plusieurs fois. Il est donc intéressant de taper un peu plus maintenant pour trouver un code plus lisible six mois plus tard.

Je suis d'accord avec ce que @GlenPeterson a dit:

doSomething(Undo.ALLOW); // call using a self evident enum

mais aussi insteresting serait le suivant puisqu'il me semble qu'il n'y a que deux possibilités (vrai ou faux)

//Two methods    

doSomething()  // this method does something but doesn't prevent undo

doSomethingPreventUndo() // this method does something and prevents undo

Puisque vous vous interrogez sur le javascript principalement, en utilisant coffeescript, vous pouvez avoir un argument nommé comme syntaxe, super facile:

# declare method, ={} declares a default value to prevent null reference errors
doSomething = ({preventUndo} = {}) ->
  if preventUndo
    undo = no
  else
    undo = yes

#call method
doSomething preventUndo: yes
#or 
doSomething(preventUndo: yes)

compile pour

var doSomething;

doSomething = function(_arg) {
  var preventUndo, undo;
  preventUndo = (_arg != null ? _arg : {}).preventUndo;
  if (preventUndo) {
    return undo = false;
  } else {
    return undo = true;
  }
};

doSomething({
  preventUndo: true
});

doSomething({
  preventUndo: true
});

Amusez-vous à http://js2coffee.org/ pour essayer les possibilités

Juste pour une solution moins conventionnelle et depuis l’OP mentionnée Javascript, une possibilité est d’utiliser un tableau associatif pour mapper des valeurs. L'avantage par rapport à un booléen unique est que vous pouvez avoir autant d'arguments que vous le souhaitez sans vous soucier de leur séquence.

var doSomethingOptions = new Array();

doSomethingOptions["PREVENTUNDO"] = true;
doSomethingOptions["TESTMODE"] = false;

doSomething( doSomethingOptions );

// ...

function doSomething( doSomethingOptions ){
    // Check for null here
    if( doSomethingOptions["PREVENTUNDO"] ) // ...
}

Je me rends compte que c’est le réflexe de quelqu'un issu d’un arrière-plan très typé et que ce n’est peut-être pas si pratique que cela, mais considérez cela comme une originalité.

Une autre possibilité est d'avoir un objet et est également possible en Javascript. Je ne suis pas sûr à 100% que ce soit parfaitement correct. Regardez des modèles tels que Factory pour plus d'inspiration.

function SomethingDoer( preventUndo ) {
    this.preventUndo = preventUndo;
    this.doSomething = function() {
            if( this.preventUndo ){
                    // ...
            }
    };
}

mySomethingDoer = new SomethingDoer(true).doSomething();

L'objectif de votre question et des autres réponses est d'améliorer la lisibilité lorsque la fonction est appelée, ce qui est un objectif avec lequel je suis d'accord. Toute directive spécifique, événement "pas d'argument booléen", doit toujours servir de moyen à cette fin et ne pas devenir et se terminer elle-même.

Je pense qu'il est utile de noter que ce problème est généralement évité lorsque le langage de programmation prend en charge les arguments nommés / arguments de mot clé, comme en C # 4 et Python, ou lorsque les arguments de méthode sont entrelacés dans le nom de la méthode, comme dans Smalltalk ou Objective-C.

Exemples:

// C# 4
foo.doSomething(preventUndo: true);

# Python
foo.doSomething(preventUndo=True)

// Objective-C
[foo doSomethingWith:bar shouldPreventUndo:YES];

Il faut éviter de passer des variables booléennes comme arguments à des fonctions. Parce que les fonctions doivent faire une chose à la fois. En passant une variable booléenne, la fonction a deux comportements. Cela crée également un problème de lisibilité à un stade ultérieur ou pour les autres programmeurs qui ont tendance à voir votre code. Cela crée également un problème lors du test de la fonction. Dans ce cas, vous devez probablement créer deux cas de test. Imaginons que si une fonction possède une instruction switch et modifie son comportement en fonction du type de commutateur, vous devez définir autant de scénarios de test différents.

Chaque fois que l'on trouve un argument booléen pour la fonction, il doit modifier le code de manière à ne pas écrire d'argument booléen.

int preventUndo = true;
doSomething(preventUndo);

Un bon compilateur pour les langages à faible niveau de niveau tels que C ++ optimisera le code machine de la ligne supérieure à 2 comme ci-dessous:

doSomething(true); 

donc, peu importe la performance, cela augmentera la lisibilité.

En outre, il est utile d’utiliser une variable au cas où elle deviendrait variable ultérieurement et accepterait également d’autres valeurs.