Puis-je ignorer en toute sécurité avertissement CodeAnalysis: remplacer la chaîne == « » avec String.IsNullOrEmpty?

StackOverflow https://stackoverflow.com/questions/1661024

  •  12-09-2019
  •  | 
  •  

Question

J'ai eu un code similaire à ceci:

string s = CreateString();
if (s == "") foo(s);

Si s est égal à "", devrait être appelé foo. Si la chaîne est nulle, ce qui ne devrait jamais arriver, puis un NullReferenceException est très bien (comme cela est, après tout, une situation exceptionnelle).

CodeAnalysis me dit de tester s.IsNullOrEmpty. Cela modifierait la fonctionnalité d'une manière nunintended.

La performance est pas un problème.

Est-il sûr de supprimer l'avertissement CA1820 associé?

Modifier. Mise à jour exemple de code et le texte afin de mieux refléter mon cas

Modifier: est (légèrement modifié) code réel (il est dans une implémentation standard IXmlSerializable):

public void ReadXml (XmlReader reader)
    // ...
    string img = reader.ReadElementString ("Image");
    if (img != "") {
        Image = Image.FromFile(img);
    }
    // ...
Était-ce utile?

La solution

Il se comportera différemment en ce qui concerne les valeurs nulles, donc cela dépend de ce que vous voulez arriver; vous mentionnez que NullReferenceException serait OK, mais il n'y a rien dans le code incriminent ce soulèverait cela, donc pourquoi il pourrait provoquer des erreurs inattendues en aval.

Vous , mais je suis toujours tenté d'ajouter:

static bool IsNullOrEmpty(this string value) {
    return string.IsNullOrEmpty(value);
}

donc je peux utiliser:

if (s.IsNullOrEmpty()) foo();

Autres conseils

Caractéristiques:

  

Si s est égal à "", devrait être appelé foo.   Si la chaîne est nulle, qui ne doit jamais   arriver, puis un NullReferenceException   est très bien.

Il suffit de tester la longueur de chaîne comme dans conseillé la règle CodeAnalysis :

if (s.Length == 0) foo(s);

Votre question:

  

Est-il sûr de supprimer l'associé   Avertissement CA1820?

Vous pouvez l'ignorer, votre code ne fonctionnera mais je ne le conseille, suivre les directives AUtANt que vous pouvez. Même si le sujet (performance) n'est pas un problème, votre code sera plus cohérente et vous obtenez l'habitude d'écrire du code standard.

Chaque avertissement d'analyse du code est la documentation associée que vous pouvez accéder en highligting l'avertissement et en appuyant sur F1 . Vous pouvez également faire un clic droit sur l'élément pour obtenir de l'aide.

Dans tous les cas, voici la documentation qui explique cet avertissement particulier .

Selon cette documentation, il est « sûr de supprimer un avertissement de cette règle si les performances ne sont pas un problème ».

Il serait préférable d'écrire le test:

if(s != null && s == "")

Vous pouvez ensuite traiter une valeur nulle dans une autre instruction if

Vous n'êtes pas ignorant vraiment l'avertissement, vous avez regardé le code et a décidé que l'avertissement ne s'applique pas. Ceci est une condition tout à fait raisonnable en vertu de laquelle pour supprimer l'avertissement.

que pure spéculation

Je voudrais savoir un peu plus sur ce que vous essayez de faire, cependant. Je pense qu'il peut y avoir une meilleure façon de le gérer. Le motif me rappelle renvoyer un message d'erreur ou vide pour signaler le succès d'une méthode. Si tel est le cas, je considérerais soit retourner vide et lancer une exception en cas d'échec ou de retour bool et seulement lancer des exceptions lorsque le message est critique et retourner true / false autrement.

Si null est OK, vous serez très bien de toute façon.

oui.

Mais je suis d'accord avec CodeAnalysis avec String.IsNullOrEmpty est un choix sûr.

Non gestion des exceptions alors que vous pouvez est gennerally une mauvaise idée si CA est juste que vous devez soit pour traiter null vide ou gérer l'exception. Une exception de référence null fait en utilisant une valeur de retour est une très mauvaise chose. À tout le moins mettre dans un Debug.Assert (s! = Null) et comparer à string.Empty

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top