Considérez la méthode suivante Signature:
public static List<Poll> GetPolls()
est ce bon style? p>
mise à jour: Disons que j'utilise la méthode suivante Signature: P>
public static bool TryGetPolls(out List<Poll> polls, out string errorMessage)
17 Réponses :
Cette méthode tente de faire trois choses différentes:
c'est assez désordonné d'un point de vue de conception.
Une meilleure approche serait de déclarer simplement: puis laissez cette méthode lancer une exception si quelque chose se passe mal.
Avoir un trygetpolls pourrait être utile s'il peut légitimement retourner.
@Michael: Les méthodes "Essayer ..." sont belles dans des cas spécifiques où quelque chose peut se tromper mais que vous ne vous souciez pas (plus ou moins). Si cela peut ne rien retourner, il ne devrait pas renverser NULL / RIEN.
Les réponses de votedisciple et de Yshuditelu sont correctes. Par exemple, INT.PARSE a également un INT.Tryparse pour des instances où vous attendez que l'analyse échoue éventuellement. Parse jette une exception, tandis que Tryparse renvoie un BOOL et utilise un param.
Je ne suis pas en désaccord, mais ma préférence dans ce cas serait une méthode Haspolls () qui renvoie simplement un booléen (ou jette une exception). Avec une mise en cache triviale, ces méthodes pourraient même être appelées successivement sans perte de performance.
+1 Pour réduire la quantité de travail nécessaire pour utiliser la méthode et pour la rendre utilisable dans une chaîne de méthode LINQ. Et il peut retourner une liste vide ou null s'il n'y a rien à retourner.
@YOODERER - Le modèle dans le cadre (Tryparse, trygetvalue, etc.) est de retourner un bool, pas null.
Notez que .NET'S'S (Numérique) .Tryparse () était une entrée tardive et créée uniquement parce qu'il était jugé un cas très courant que l'analyse échouerait. Ce n'est pas considéré comme une norme-cadre de modéliser et plutôt d'une implémentation spécifique pour un problème spécifique (mais commun).
Si vous pouvez tester et retourner une valeur au lieu de jeter des exceptions à gauche et à droite, je crois que ce serait mieux.
@Snorfus: Notez également que lorsque l'échec est commun, Tryparse est plus rapide, car elle évite un lancer. Oui, les gens ont comparu ceci. Et oui, cela n'a souvent pas d'importance.
Pourquoi avez-vous besoin d'essayer d'abord? S'il n'y a pas de records, la collection devrait simplement être vide.
+1 ... Je suis d'accord, je suis en désordre ... Outre les méthodes Tryparse ou TryGet ou TryBLAH sont généralement utilisées parce que vous Ne voulez pas savoir sur les erreurs. Si vous recherchez un message d'erreur, appelez la version non essentielle de la méthode et gérez les exceptions!
Je le ferais iSenumerable
(Aussi, dans tous les cas où des éléments ne pouvaient être légitimement retournés, il est plus intuitif et moins difficile de renvoyer une liste vide plutôt que de revenir null.)
Comparez le code de la version qui renvoie une version BOOL et la version qui jette une exception. Vous trouverez qu'ils sont sur le même niveau de verbosité. Les exceptions en C # sont utilisées pour indiquer que la classe ne pouvait pas faire ce qui lui a été demandé, , que cela soit ou non considéré comme exceptionnel . Puisque vous n'appelez probablement pas aussi souvent cette méthode, vous êtes probablement bien pour la performance. Et le «si HasRecords () .. GetRecords ()» Type de solution est soumis à toutes sortes de conditions de course.
Je crois que serait plus approprié. Si la méthode est un tryget , mon hypothèse initiale serait la raison de s'attendre à ce que cela échoue et que cela soit sur l'appelant pour déterminer quoi faire ensuite. Si leur appelant ne manipule pas l'erreur, sinon souhaite des informations d'erreur, je m'attendrais à ce qu'ils appellent une méthode correspondante .
Encore mieux, utilisez un ilon
En règle générale, je dirais non.
La raison pour laquelle je dis non n'est en réalité pas parce que vous effectuez un Le Au lieu de cela, ce que vous recherchez est une paire de méthodes: de cette façon, l'utilisateur peut faire ce qui est approprié et trygetx et retourner un bool avec un paramètre out . Je pense que c'est mauvais style parce que vous retournez également une chaîne d'erreur. Essayez doit seulement ignorer une erreur spécifique et couramment rencontrée. D'autres problèmes peuvent toujours jeter une exception avec le message d'exception approprié. N'oubliez pas que le but d'un essayer comme ceci est d'éviter les frais généraux d'une exception lancée lorsque vous vous attendez à une sorte de défaut unique qui ne se produise pas plus fréquemment que non. getpolls peut être implémenté dans termes de trygetpolls . Je suppose que votre statique est un sens dans le contexte.
envisager de revenir:
Multiple Out Paramètres, à moi, est un odeur de code . La méthode ne devrait faire qu'une chose uniquement.
envisager de collecter et de manipuler des messages d'erreur avec:
+1: envisagez également de mettre en œuvre une exception personnalisée pour le cas connu où il échouera, étant donné certaines conditions.
Mais vous devriez jeter quelque chose de plus spécifique qu'un système.Exception qui correspond à ce type d'erreur plus précisément.
Merci Ian. Il s'agissait de plus en ligne qu'une exception d'une certaine sorte devrait être lancée. N'était pas censé être une copie littérale / pâte de la paroi du code de la salle de bain. :)
Cela dépend de ce que le message d'erreur est. Par exemple, si le traitement ne pouvait pas continuer car la connexion de base de données n'était pas disponible, etc., vous devriez alors jeter une exception lorsque d'autres personnes ont mentionné.
Cependant, il se peut que vous souhaitiez simplement retourner "Meta" des informations sur la tentative, auquel cas avez-vous juste besoin d'un moyen de retourner plus d'une information d'un seul appel de méthode. Dans ce cas, je suggère de faire une classe de polresponse contenant deux propriétés: liste
C'est comme un-idiomatique comme le code d'origine, IMHO. Idiomatic c # utilise des exceptions pour signaler les erreurs, non des objets personnalisés avec des messages d'erreur.
La technique de Scott peut être similaire à System.Web.httpresponse Je ne pense pas que ce soit une hypothèse injuste que la gestion d'un recueil d'objets de sondage pourrait justifier un objet entièrement nouveau qui gère les erreurs, les langues, l'expiration, le niveau de visibilité des utilisateurs, etc.
@Greg - je pense que cela revient à ce que c'est vraiment errorMessage. Tout le monde ici est juste en supposant que c'est une exception de programme, mais ce n'est pas nécessairement le cas. Une exception de programme signifie que l'exécution ne pouvait pas continuer. Une exception et une erreur ne sont pas la même chose. Lorsqu'un utilisateur tente de saisir un caractère dans un champ de numéro, c'est une erreur, mais je n'ai pas à jeter une exception. Peut-être dans ce cas que "l'erreur" est que les données de sondage sont obsolètes, ou peut-être que les résultats ne s'ajoutent pas à 100%. Je pensais plus dans les lignes de "Comment puis-je retourner deux choses d'un appel de la fonction?"
@JIM Ce n'est pas exactement une httpesponse dans la question.
@Stuart - Je ne m'incline pas à la pression des pairs. Il n'y a rien de mal à cette méthode dans des circonstances appropriées. Il est parfaitement légitime de retourner des métadonnées avec une demande de «obtenir» quelque chose sur ce qui s'est passé pendant le processus.
@Scott - Je suis tout à fait d'accord pour dire qu'il est souvent utile de renvoyer des métadonnées supplémentaires avec un résultat (nous le faisons tout le temps, par exemple, avoir une interface d'IsearchResult
@Greg - vous êtes probablement correct. Je vais laisser cela en haut parce que je pense que, contrairement au reste des réponses de moi, celui-ci ajoute une idée supplémentaire, pour que quiconque dispose de faire défiler jusqu'au bas de la page. Nous essayons de créer une base de données interrogeable de questions / réponses ici pour référence future, pas seulement répondre à la question de cette personne.
@Scott - avoir un vote up-vote ... au moins cela pourrait vous cogner morts bas :-)
dépend de si une erreur est une occurrence courante ou si elle nous a vraiment une exception.
Si des erreurs sont gunuinement rares et mauvaises, vous voudrez peut-être envisager la méthode de renvoyer la liste des sondages et lancez une exception si une erreur se produit.
Si une erreur est quelque chose qui est une partie commune de manière réelle des opérations normales, comme une erreur couvrant une chaîne à un entier dans la méthode Int.Tryparse, la méthode que vous avez créée serait plus appropriée.
Je suppose que le premier est probablement le meilleur cas pour vous.
Je suis d'accord avec tous vos raisons, sauf que dans le 2e cas, j'utiliserais les conseils de Yshuditelu, pas le code d'origine.
Je ne pense pas que "rareté" est le bon concept. "Exceptionnel" est plus précis. Considérez une fonction acceptant un numéro comme une chaîne comme argument. Si la chaîne n'est pas un nombre, c'est un cas exceptionnel, mais pas nécessairement un rare.
Ceci n'est certainement pas une manière idiomatique d'écriture C #, ce qui signifierait également que ce n'était probablement pas un bon style.
Lorsque vous avez un Lorsque vous avez simplement une méthode getpolls alors cela signifie que cela signifie que cela signifie Vous voulez toujours les résultats et si cela ne réussit pas, vous voulez savoir pourquoi sous la forme d'une exception Mélanger les deux est quelque part entre les deux, ce qui sera être inhabituel pour la plupart des gens. Donc, je dirais que je ne retournerais pas non plus le message d'erreur, soit lancer une exception Donc, votre signature de méthode devrait donc probablement Soit: ou (Notez que je retourne un trygetpolls , alors cela signifie que vous voulez que vous souhaitiez les résultats si l'opération réussit, et si ce n'est pas alors que vous ne vous souciez pas de la raison pour laquelle il ne réussit pas. sur une défaillance, mais n'utilisez pas cette approche hybride étrange. iliste
Je suis tout pour l'abstraction dans une API publique spécialement si le client n'est pas dans votre contrôle. Mais s'il s'agit d'une classe interne, quelle est la probabilité que vous remplaciez la mise en œuvre de la liste dans la framework .NET avec autre chose. Si c'était une interface que vous pouvez vous moquer de suffisamment, mais la classe de liste .NET? Que diriez-vous du principe de Kiss.
@Pratik - Comment est un
@Greg - il n'est pas du tout complexe pour la classe de déclaration. Mais pour les classes internes où vous codez l'utilisateur de la classe, vous devez souvent créer une liste à nouveau, utilisez AddRange, puis vous pouvez utiliser la méthode de la liste des recherches. Le retour d'une liste simplifie directement cela. Je suis curieux de savoir quelle classe de collecte vous avez utilisée pour remplacer la liste
Pourquoi utiliseriez-vous la liste
Non, de mon point de vue c'est un style très mauvais. Je l'écrirais comme ceci: si l'appel échoue, lancez une exception et placez le message d'erreur à l'exception. C'est quelles exceptions sont pour et votre code deviendra beaucoup plus propre, plus lisible et plus facile à entretenir.
Cela dépend de la manière dont la méthode échouera fréquemment. En général, les erreurs de .net doivent être communiquées avec une exception Pour le type de base de données, je pense qu'un . Le cas où cette règle ne tient pas est lorsque la condition d'erreur est fréquente et que l'impact de la performance de jette et exception est trop élevé.
exception est le meilleur.
Je le reporterais comme ceci. Il devrait probablement lancer une exception (l'erroressage) si elle ne parvient pas à extraire les sondages, plus cela permet une chaîne de méthode qui est moins Étailé que de traiter des paramètres Out. Si vous exécutez FXCop, vous voudrez changer de liste sur IList pour le garder heureux.
Pas vraiment - je peux voir un certain nombre de problèmes avec cela.
Tout d'abord, la méthode sonne comme si vous vous attendriez normalement à ce qu'elle réussisse; Erreurs (Impossible de se connecter à la base de données, impossible d'accéder à la table etc.) serait rare. Dans ce cas, il est beaucoup plus raisonnable d'utiliser des exceptions pour signaler les erreurs. Le modèle Deuxièmement, vous signalez des erreurs sous forme de valeur Pour résumer: Utilisez des exceptions plutôt que est pour les cas où vous vous attendez souvent à l'appel à "échouer" - E.G. Lors de l'analyse d'une chaîne à un entier, les chances sont bonnes que la chaîne est une entrée de l'utilisateur qui peut être invalide, vous devez donc avoir un moyen rapide de gérer cela - donc trypetse . Ce n'est pas le cas ici.
BOOL indiquant la présence ou l'absence d'erreur, et un message de chaîne. Comment l'appelant distingue-t-il différentes erreurs alors? Il ne peut certainement pas correspondre au texte du message d'erreur - c'est un détail de mise en œuvre qui est sujet à changement et peut être localisé. Et il pourrait y avoir un monde de différence entre quelque chose comme "Impossible de se connecter à la base de données" (peut-être simplement ouvrir la boîte de dialogue Paramètres de connexion de base de données dans ce cas et laisser l'utilisateur le modifier?) Et "connecté à la base de données, mais" l'accès refusé " ". Votre API ne donne aucun bon moyen de faire la distinction entre celles-ci.
BOOL + OUT String Pour signaler les messages. Une fois que vous l'avez fait, vous pouvez simplement utiliser list out argument. Et, bien sûr, renommez la méthode sur getpolls , puisque Essayez ... est réservé pour bool + out Modèle.
Je pense que c'est bien. Je préférerais cependant: afin que les chaînes d'erreur ne vivent pas dans le code d'accès aux données ...
C # Les méthodes ne doivent vraiment faire qu'une seule chose. Vous essayez de faire trois choses avec cette méthode. Je ferais aussi que d'autres ont suggéré et lancé une exception s'il y a une erreur. Une autre option serait de créer des méthodes d'extension pour votre objet de liste.
E.g. Dans une classe statique publique: P>
List<Poll> polls = new List<Poll>().Fill();
if(polls != null)
{
// no errors occur
}
Veuillez indiquer vos hypothèses, contraintes, désirs / objectifs et raisonnement; Nous devons deviner et / ou lire votre esprit pour savoir quelles sont vos intentions.
supposer que vous voulez que votre fonction
La signature ci-dessus est bien (bien que la déglutition, toutes les exceptions possibles ne soit pas une bonne pratique).
En tant que style de codage général, il a des problèmes potentiels, car d'autres ont mentionné.
Les lignes directrices disent à essayer d'éviter Renvoyer également les codes d'erreur ou les messages ne constitue pas une meilleure pratique, la meilleure pratique est d'utiliser Un meilleur moyen de déclarer la méthode est comme celle-ci. et pour la signification des erreurs utilise une manipulation d'exception
Il y a aussi ce modèle, comme on le voit dans de nombreuses fonctions Win32.
public static bool GetPolls(out List<Poll> polls) if(!PollStuff.GetPolls(out myPolls)) string errorMessage = PollStuff.GetLastError();
Est-ce que tu me manques quelque chose ici? L'asker de la question semble vouloir savoir comment nettoyer les ressources si la méthode échoue.
public static IList<Poll> GetPolls()
{
try
{
}
finally
{
// check that the connection happened before exception was thrown
// dispose if necessary
// the exception will still be presented to the caller
// and the program has been set back into a stable state
}
}
Pour formater le code, ajoutez quatre espaces avant de cela. La méthode la plus simple consiste à taper / copier votre code, à tout mettre en surbrillance, puis à cliquer sur le bouton "Code" de l'éditeur.