Spécifiez des noms de paramètres facultatifs même s'ils ne sont pas requis?

25

Considérez la méthode suivante:

public List<Guid> ReturnEmployeeIds(bool includeManagement = false)
{

}

Et l'appel suivant:

var ids = ReturnEmployeeIds(true);

Pour un développeur nouveau dans le système, il serait assez difficile de deviner ce qui trues'est passé. La première chose que vous feriez serait de survoler le nom de la méthode ou d'aller à la définition (qui ne sont en aucun cas de grosses tâches). Mais, par souci de lisibilité, est-il logique d'écrire:

var ids = ReturnEmployeeIds(includeManagement: true);

Y a-t-il quelque part qui, officiellement, discute de la spécification explicite ou non des paramètres facultatifs lorsque le compilateur n'en a pas besoin?

L'article suivant décrit certaines conventions de codage: https://msdn.microsoft.com/en-gb/library/ff926074.aspx

Quelque chose de similaire à l'article ci-dessus serait formidable.

JᴀʏMᴇᴇ
la source
2
Ce sont deux questions plutôt indépendantes: (1) Devriez-vous écrire des arguments facultatifs pour plus de clarté? (2) Faut-il utiliser des booleanarguments de méthode, et comment?
Kilian Foth
5
Tout cela se résume à une préférence / style / opinion personnelle. Personnellement, j'aime les noms de paramètres lorsque la valeur transmise est un littéral (une variable peut déjà transmettre suffisamment d'informations via son nom). Mais c'est mon opinion personnelle.
MetaFight
1
peut-être inclure ce lien comme exemple de source dans votre question?
MetaFight
4
Si cela ajoute quelque chose, je l'ai exécuté via StyleCop & ReSharper - ni l'un ni l'autre n'avait une vision claire à ce sujet. ReSharper dit simplement que le paramètre nommé pourrait être supprimé, puis continue en disant qu'un paramètre nommé pourrait être ajouté. Les conseils sont gratuits pour une raison que je suppose ...: - /
Robbie Dee

Réponses:

28

Je dirais que dans le monde C #, une énumération serait l'une des meilleures options ici.

Avec cela, vous seriez obligé de préciser ce que vous faites et tout utilisateur verrait ce qui se passe. Il est également sûr pour les futures extensions;

public enum WhatToInclude
{
    IncludeAll,
    ExcludeManagement
}

var ids = ReturnEmployeeIds(WhatToInclude.ExcludeManagement);

Donc, à mon avis ici:

énum> option énum> option bool

Edit: En raison de la discussion avec LeopardSkinPillBoxHat ci-dessous concernant une [Flags]énumération qui dans ce cas spécifique pourrait être un bon ajustement (car nous parlons spécifiquement d'inclure / exclure des choses), je propose plutôt d'utiliser ISet<WhatToInclude>comme paramètre.

C'est un concept plus "moderne" avec plusieurs avantages, provenant principalement du fait qu'il s'intègre dans la famille de collections LINQ, mais aussi qui [Flags]comprend un maximum de 32 groupes. Le principal inconvénient de l'utilisation ISet<WhatToInclude>est la mauvaise prise en charge des ensembles dans la syntaxe C #:

var ids = ReturnEmployeeIds(
    new HashSet<WhatToInclude>(new []{ 
        WhatToInclude.Cleaners, 
        WhatToInclude.Managers});

Certains d'entre eux peuvent être atténués par des fonctions d'assistance, à la fois pour générer l'ensemble et pour créer des "ensembles de raccourcis" tels que

var ids = ReturnEmployeeIds(WhatToIncludeHelper.IncludeAll)
NiklasJ
la source
J'ai tendance à être d'accord; et ont généralement pris l'habitude d'utiliser des énumérations dans mon nouveau code s'il y a une chance vague que les conditions soient étendues à l'avenir. Remplacer un booléen par une énumération lorsque quelque chose devient un tri-état finit par créer des différences vraiment bruyantes et rendre le code de blâme beaucoup plus fastidieux; lorsque vous finirez par devoir modifier votre code à nouveau un an plus tard pour une troisième option.
Dan Neely
3
J'aime l' enumapproche, mais je ne suis pas un grand fan de mixage Includeet Excludede même enumqu'il est plus difficile de comprendre ce qui se passe. Si vous voulez que cela soit vraiment extensible, vous pouvez en faire un [Flags] enum, les faire tous IncludeXxxet fournir un IncludeAllqui est défini sur Int32.MaxValue. Ensuite, vous pouvez fournir 2 ReturnEmployeeIdssurcharges - une qui renvoie tous les employés et une qui prend un WhatToIncludeparamètre de filtre qui peut être une combinaison d'indicateurs enum.
LeopardSkinPillBoxHat
@LeopardSkinPillBoxHat Ok, je suis d'accord sur l'exclusion / inclusion. Donc, c'est un peu un exemple artificiel, mais j'aurais pu l'appeler IncludeAllButManagement. Sur votre autre point, je ne suis pas d'accord - je pense que [Flags]c'est une relique et ne devrait être utilisé que pour des raisons d'héritage ou de performance. Je pense que ISet<WhatToInclude>c'est un concept bien meilleur (malheureusement, C # manque de sucre syntaxique pour le rendre agréable à utiliser).
NiklasJ
@NiklasJ - J'aime l' Flagsapproche car elle permet à l'appelant de passer WhatToInclude.Managers | WhatToInclude.Cleanerset au niveau du bit ou exprime exactement comment l'appelant la traitera (c'est-à-dire les gestionnaires ou les nettoyeurs). Sucre syntaxique intuitif :-)
LeopardSkinPillBoxHat
@LeopardSkinPillBoxHat Exactement, mais c'est le seul avantage de cette méthode. Les inconvénients incluent par exemple un nombre limité de choix et ne sont pas compatibles avec d'autres concepts de type linq.
NiklasJ
20

est-il logique d'écrire:

 var ids=ReturnEmployeeIds(includeManagement: true);

Il est discutable s'il s'agit d'un "bon style", mais à mon humble avis, ce n'est pas du moins un mauvais style, et utile, tant que la ligne de code ne devient pas "trop ​​longue". Sans paramètres nommés, c'est également un style courant d'introduire une variable explicative:

 bool includeManagement=true;
 var ids=ReturnEmployeeIds(includeManagement);

Ce style est probablement plus courant chez les programmeurs C # que la variante de paramètre nommé, car les paramètres nommés ne faisaient pas partie du langage avant la version 4.0. L'effet sur la lisibilité est presque le même, il a juste besoin d'une ligne de code supplémentaire.

Donc, ma recommandation: si vous pensez que l'utilisation d'une énumération ou de deux fonctions avec des noms différents est "excessive", ou si vous ne voulez pas ou ne pouvez pas changer la signature pour une raison différente, allez-y et utilisez le paramètre nommé.

Doc Brown
la source
Pourrait noter que vous utilisez de la mémoire supplémentaire sur le deuxième exemple
Alvaro
10
@Alvaro Il est très peu probable que cela soit vrai dans un cas aussi trivial (où la variable a une valeur constante). Même si c'était le cas, cela ne vaut guère la peine d'être mentionné. La plupart d'entre nous ne fonctionnent pas sur 4 Ko de RAM ces jours-ci.
Chris Hayes
3
Comme il s'agit de C #, vous pouvez marquer en includeManagementtant que local constet éviter d'utiliser du tout de mémoire supplémentaire.
Jacob Krall
8
Les gars, je ne vais pas ajouter quelque chose à ma réponse qui pourrait seulement distraire de ce que je pense est important ici.
Doc Brown
17

Si vous rencontrez du code comme:

public List<Guid> ReturnEmployeeIds(bool includeManagement = false)
{

}

vous pouvez à peu près garantir que ce qui se trouve à l'intérieur {}sera quelque chose comme:

public List<Guid> ReturnEmployeeIds(bool includeManagement = false)
{
    if (includeManagement)
        return something
    else
        return something else
}

En d'autres termes, le booléen permet de choisir entre deux modes d'action: la méthode a deux responsabilités. C'est à peu près garanti une odeur de code . Nous devons toujours nous efforcer de garantir que les méthodes ne font qu'une seule chose; ils n'ont qu'une seule responsabilité. Par conséquent, je dirais que vos deux solutions sont la mauvaise approche. L'utilisation de paramètres facultatifs est un signe que la méthode fait plus d'une chose. Les paramètres booléens en sont également le signe. Les deux devraient définitivement sonner l'alarme. Par conséquent, vous devez refactoriser les deux ensembles de fonctionnalités différents en deux méthodes distinctes. Donnez aux méthodes des noms qui indiquent clairement ce qu'elles font, par exemple:

public List<Guid> GetNonManagementEmployeeIds()
{

}

public List<Guid> GetAllEmployeeIds()
{

}

Cela améliore à la fois la lisibilité du code et garantit que vous suivez mieux les principes SOLID.

EDIT Comme cela a été souligné dans les commentaires, le cas ici que ces deux méthodes auront probablement des fonctionnalités partagées, et que les fonctionnalités partagées seront probablement mieux enveloppées dans une fonction privée qui aura besoin d'un paramètre booléen pour contrôler certains aspects de ce qu'elle fait .

Dans ce cas, le nom du paramètre doit-il être fourni, même si le compilateur n'en a pas besoin? Je dirais qu'il n'y a pas de réponse simple à cela et qu'un jugement est nécessaire au cas par cas:

  • L'argument pour spécifier le nom est que d'autres développeurs (vous y compris dans six mois) devraient être le public principal de votre code. Le compilateur est définitivement un public secondaire. Il faut donc toujours écrire du code pour faciliter la lecture aux autres; plutôt que de simplement fournir ce dont le compilateur a besoin. Un exemple de la façon dont nous utilisons cette règle tous les jours, c'est que nous ne remplissons pas notre code avec les variables _1, _2 etc., nous utilisons des noms significatifs (qui ne signifient rien pour le compilateur).
  • Le contre-argument est que les méthodes privées sont des détails d'implémentation. On pourrait raisonnablement s'attendre à ce que quiconque regarde une méthode comme celle ci-dessous, trace à travers le code pour comprendre le but du paramètre booléen car ils sont à l'intérieur des internes du code:
public List<Guid> GetNonManagementEmployeeIds()
{
    return ReturnEmployeeIds(true);
}

Le fichier source et les méthodes sont-ils petits et faciles à comprendre? Si oui, le paramètre nommé équivaut probablement à du bruit. Sinon, cela pourrait être très utile. Appliquez donc la règle selon les circonstances.

David Arno
la source
12
J'entends ce que vous dites, mais vous avez en quelque sorte manqué le point de ce que je demande. En supposant un scénario dans lequel il est plus approprié d'utiliser des paramètres facultatifs (ces scénarios existent), est-il acceptable de nommer les paramètres même si cela n'est pas nécessaire?
JᴀʏMᴇᴇ
4
C'est vrai, mais la question portait sur un appel à une telle méthode. De plus, un paramètre indicateur ne garantit pas du tout une branche dans la méthode. Il peut simplement être transmis à une autre couche.
Robbie Dee
Ce n'est pas faux, mais trop simpliste. Dans le code réel, vous trouverez public List<Guid> ReturnEmployeeIds(bool includeManagement) { calculateSomethingHereForBothBranches; if (includeManagement) return something else return something else }, donc le fractionnement que simplement en deux méthodes signifiera probablement que ces deux méthodes auront besoin du résultat du premier calcul comme paramètre.
Doc Brown
4
Je pense que ce que nous disons tous, c'est que le visage public de votre classe devrait probablement exposer deux méthodes (chacune avec une responsabilité) mais, en interne, chacune de ces méthodes peut raisonnablement transmettre la demande à une seule méthode privée avec un paramètre bool de branchement .
MetaFight
1
Je peux imaginer 3 cas pour le comportement qui diffère entre les deux fonctions. 1. Le prétraitement est différent. Demandez aux fonctions publiques de prétraiter les arguments dans la fonction privée. 2. Il y a un post-traitement différent. Demandez aux fonctions publiques de post-traiter le résultat de la fonction privée. 3. Il existe un comportement intermédiaire qui diffère. Cela peut être transmis en tant que lambda à la fonction privée, en supposant que votre langue le prend en charge. Souvent, cela conduit à une nouvelle abstraction réutilisable qui ne vous était pas vraiment venue auparavant.
jpmc26
1

Pour un développeur nouveau dans le système, il serait assez difficile de deviner ce qui s'est passé. La première chose que vous feriez serait de passer le curseur sur le nom de la méthode ou d'aller à la définition (qui ne sont en aucun cas de grosses tâches)

Oui c'est vrai. Le code d'auto-documentation est une belle chose. Comme d'autres réponses l'ont souligné, il existe des moyens de lever l'ambiguïté de l'appel à ReturnEmployeeIdsen utilisant une énumération, des noms de méthode différents, etc. Cependant, parfois, vous ne pouvez pas vraiment éviter le cas, mais vous ne voulez pas vous en aller et utiliser les partout (sauf si vous aimez la verbosité de Visual Basic).

Par exemple, cela peut aider à clarifier un appel mais pas nécessairement un autre.

Un argument nommé peut être utile ici:

var ids = ReturnEmployeeIds(includeManagement: true);

N'ajoute pas beaucoup de clarté supplémentaire (je suppose que cela analyse une énumération):

Enum.Parse(typeof(StringComparison), "Ordinal", ignoreCase: true);

Cela peut en fait réduire la clarté (si une personne ne comprend pas la capacité d'une liste):

var employeeIds = new List<int>(capacity: 24);

Ou là où cela n'a pas d'importance parce que vous utilisez de bons noms de variables:

bool includeManagement = true;
var ids = ReturnEmployeeIds(includeManagement);

Y a-t-il quelque part qui, officiellement, discute de la spécification explicite ou non des paramètres facultatifs lorsque le compilateur n'en a pas besoin?

Pas AFAIK. Abordons les deux parties cependant: la seule fois où vous devez utiliser explicitement les paramètres nommés est parce que le compilateur vous oblige à le faire (généralement c'est pour supprimer l'ambiguïté des instructions). Donc, à part ça, vous pouvez les utiliser quand vous le souhaitez. Cet article de MS contient des suggestions sur le moment où vous pouvez les utiliser, mais ce n'est pas particulièrement définitif https://msdn.microsoft.com/en-us/library/dd264739.aspx .

Personnellement, je trouve que je les utilise le plus souvent lorsque je crée des attributs personnalisés ou que je stoppe une nouvelle méthode où mes paramètres peuvent changer ou être réorganisés (les attributs nommés b / c peuvent être répertoriés dans n'importe quel ordre). De plus, je ne les utilise que lorsque je fournis une valeur statique en ligne, sinon si je passe une variable, j'essaie d'utiliser de bons noms de variables.

TL; DR

En fin de compte, cela se résume à la préférence personnelle. Bien sûr, il existe quelques cas d'utilisation lorsque vous devez utiliser des paramètres nommés, mais après cela, vous devez effectuer un appel de jugement. IMO - Utilisez-le partout où vous pensez qu'il vous aidera à documenter votre code, à réduire l'ambiguïté ou à vous permettre de protéger les signatures de méthode.


la source
0

Si le paramètre est évident d'après le nom (complet) de la méthode et que son existence est naturelle à ce que la méthode est censée faire, vous ne devez pas utiliser la syntaxe de paramètre nommée. Par exemple, dans ReturnEmployeeName(57)c'est assez sacrément évident que 57c'est l'ID de l'employé, il est donc redondant de l'annoter avec la syntaxe de paramètre nommée.

Avec ReturnEmployeeIds(true), comme vous l' avez dit, il est impossible à moins que vous regardez la déclaration / documentation de la fonction à la figure ce que des truemoyens - de sorte que vous devez utiliser la syntaxe de paramètre nommé.

Maintenant, considérons ce troisième cas:

void PrintEmployeeNames(bool includeManagement) {
    foreach (id; ReturnEmployeeIds(includeManagement)) {
        Console.WriteLine(ReturnEmployeeName(id));
    }
}

La syntaxe du paramètre nommé n'est pas utilisée ici, mais puisque nous transmettons une variable à ReturnEmployeeIds, et que cette variable a un nom, et que ce nom signifie la même chose que le paramètre auquel nous le transmettons (c'est souvent le cas - mais pas toujours!) - nous n'avons pas besoin de la syntaxe de paramètre nommée pour comprendre sa signification à partir du code.

Donc, la règle est simple - si vous pouvez facilement comprendre à partir de l' appel de méthode ce que le paramètre est censé signifier, n'utilisez pas le paramètre nommé. Si vous ne pouvez pas - c'est une lacune dans la lisibilité du code, et vous voulez probablement les corriger (pas à n'importe quel prix, bien sûr, mais vous voulez généralement que le code soit lisible). Le correctif n'a pas besoin d'être nommé paramètres - vous pouvez également mettre la valeur dans une variable en premier, ou utiliser les méthodes suggérées dans les autres réponses - tant que le résultat final permet de comprendre facilement ce que fait le paramètre.

Idan Arye
la source
7
Je suis en désaccord, ce qui ReturnEmployeeName(57)rend immédiatement évident qu'il 57s'agit de la pièce d'identité. GetEmployeeById(57)d'autre part ...
Hugo Zink
@HugoZink Au départ, je voulais utiliser quelque chose comme ça pour l'exemple, mais je l'ai intentionnellement changé ReturnEmployeeNamepour montrer des cas où même sans mentionner explicitement le paramètre dans le nom de la méthode, il est assez raisonnable d'attendre des gens qu'ils le comprennent. Quoi qu'il en soit, il est à noter que contrairement à ReturnEmployeeName, vous ne pouvez pas raisonnablement renommer ReturnEmployeeIdsquelque chose qui indique la signification des paramètres.
Idan Arye
1
Dans tous les cas, il est peu probable que nous écrivions ReturnEmployeeName (57) dans un vrai programme. Il est beaucoup plus probable que nous écrivions ReturnEmployeeName (employeeId). Pourquoi devrions-nous coder en dur un ID d'employé? À moins que ce ne soit l'ID du programmeur et qu'il y ait une sorte de fraude, par exemple, ajoutez un peu au salaire de cet employé. :-)
Jay
0

Oui, je pense que c'est du bon style.

Cela a plus de sens pour les constantes booléennes et entières.

Si vous passez une variable, le nom de la variable devrait clarifier la signification. Si ce n'est pas le cas, vous devriez donner à la variable un meilleur nom.

Si vous passez une chaîne, la signification est souvent claire. Comme si je vois un appel à GetFooData ("Oregon"), je pense qu'un lecteur peut à peu près deviner que le paramètre est un nom d'état.

Bien sûr, toutes les chaînes ne sont pas évidentes. Si je vois GetFooData ("M"), sauf si je connais la fonction, je n'ai aucune idée de ce que signifie "M". S'il s'agit d'une sorte de code, comme M = "employés de niveau de gestion", nous devrions probablement avoir une énumération plutôt qu'un littéral, et le nom de l'énumération doit être clair.

Et je suppose qu'une chaîne pourrait être trompeuse. Peut-être que "Oregon" dans mon exemple ci-dessus fait référence, non pas à l'État, mais à un produit ou à un client ou autre.

Mais GetFooData (true) ou GetFooData (42) ... cela pourrait signifier n'importe quoi. Les paramètres nommés sont une merveilleuse façon de le rendre auto-documenté. Je les ai utilisés exactement à cette fin à plusieurs reprises.

Geai
la source
0

Il n'y a pas de raisons très claires pour lesquelles utiliser ce type de syntaxe en premier lieu.

En général, essayez d'éviter d'avoir des arguments booléens qui à distance peuvent sembler arbitraires. includeManagementaffectera (très probablement) le résultat. Mais l'argument semble avoir «peu de poids».

L'utilisation d'une énumération a été discutée, non seulement elle ressemblera à l'argument "porte plus de poids", mais elle fournira également une évolutivité à la méthode. Cela pourrait cependant ne pas être la meilleure solution dans tous les cas, car votre ReturnEmployeeIdsméthode devra évoluer parallèlement au WhatToInclude-enum (voir la réponse de NiklasJ ). Cela pourrait vous donner mal à la tête plus tard.

Considérez ceci: si vous WhatToIncludemodifiez l'échelle -enum, mais pas la ReturnEmployeeIdsméthode -méthode. Il pourrait alors jeter un ArgumentOutOfRangeException(meilleur cas) ou retourner quelque chose de complètement indésirable ( nullou un vide List<Guid>). Ce qui dans certains cas peut dérouter le programmeur, surtout si vous ReturnEmployeeIdsêtes dans une bibliothèque de classes, où le code source n'est pas facilement disponible.

On supposera que WhatToInclude.Traineescela fonctionnera si WhatToInclude.Allcela fonctionne, étant donné que les stagiaires sont un "sous-ensemble" de tous.

Cela dépend (bien sûr) de la façon dont il ReturnEmployeeIds est mis en œuvre.


Dans les cas où un argument booléen pourrait être transmis, j'essaie plutôt de le diviser en deux méthodes (ou plus, si nécessaire), dans votre cas; on pourrait extraire ReturnAllEmployeeIds, ReturnManagementIdset ReturnRegularEmployeeIds. Cela couvre toutes les bases et est absolument explicite à quiconque le met en œuvre. Cela n'aura pas non plus le problème de "mise à l'échelle" mentionné ci-dessus.

Puisqu'il n'y a que deux résultats pour quelque chose qui a un argument booléen. La mise en œuvre de deux méthodes nécessite très peu d'efforts supplémentaires.

Moins de code, c'est rarement mieux.


Cela dit, il existe certains cas où la déclaration explicite de l'argument améliore la lisibilité. Considérez par exemple. GetLatestNews(max: 10). GetLatestNews(10)est encore assez explicite, la déclaration explicite de maxcontribuera à dissiper toute confusion.

Et si vous devez absolument avoir un argument booléen, quel usage ne peut pas être déduit en lisant simplement trueou false. Alors je dirais que:

var ids = ReturnEmployeeIds(includeManagement: true);

.. est absolument meilleur et plus lisible que:

var ids = ReturnEmployeeIds(true);

Puisque dans le deuxième exemple, le truepourrait signifier absolument n'importe quoi . Mais j'essaierais d'éviter cet argument.

mourir maus
la source