Puis-je ignorer en toute sécurité CodeAnalysis warning: remplacer string == "" par string.IsNullOrEmpty?

Question

J'ai obtenu 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, alors une exception NullReferenceException est bonne (car c'est, après tout, une situation exceptionnelle). CodeAnalyse me dit de tester pour s.IsNullOrEmpty.

Cela modifierait la fonctionnalité de manière non intentionnelle.

Les performances ne sont pas un problème.

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

Éditer: Échantillon de code mis à jour et texte pour mieux refléter mon cas.

Edit: Ce est le (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); 
    } 
    // ... 

Answer 1

Il se comportera différemment en ce qui concerne les valeurs nulles, donc cela dépend de ce que vous vouloir arriver; vous mentionnez que NullReferenceException serait OK, mais il n'y a rien dans le code cité qui soulèverait cela, d'où la raison pour laquelle il pourrait causer des erreurs inattendues en aval.

Je ne ont, 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(); 

Answer 2

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

Answer 3

oui.

Mais je serais d'accord avec CodeAnalysis avec string.IsnullOrEmpty est un choix sûr.

Answer 4

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

Answer 5

avertissement Chaque analyse du code a la documentation associée que vous pouvez accéder par en surlignant l'avertissement et en appuyant sur F1. Vous pouvez également cliquer avec le bouton droit sur l'élément pour obtenir de l'aide.

Dans tous les cas, voici le documentation that explains that particular warning. Selon cette documentation, il est "prudent de supprimer un avertissement de cette règle si la performance n'est pas un problème".

Answer 6

Ne pas gérer les exceptions alors que vous pouvez être génériquement une mauvaise idée, alors CA a raison de dire que vous devez traiter null comme vide ou gérer l'exception. Une exception de référence nulle provoquée par l'utilisation d'une valeur de retour est une très mauvaise chose. Au moins mettre dans un Debug.Assert (s! = Null) et comparer à la chaîne.Vide

Answer 7

Vous n'ignorez pas vraiment l'avertissement, vous avez examiné le code et décidé que l'avertissement ne s'applique pas. C'est une condition parfaitement raisonnable pour supprimer l'avertissement.

que pure spéculation

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

Answer 8

Spécifications:

Si s est égal à "" devrait être appelé foo. Si la chaîne est nulle, ce qui ne devrait jamais se produire, alors une exception NullReferenceException est correcte.

Juste test the string length as adviced in the CodeAnalysis rule:

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 autant les directives que vous pouvez. Même si le sujet (performance) n'est pas un problème, votre code sera plus cohérent et vous vous habituerez à écrire du code standard.