Могу ли я безопасно игнорировать предупреждение CodeAnalysis: заменить string == "" на string.IsNullOrEmpty? - PullRequest
3 голосов
/ 02 ноября 2009

У меня есть код, подобный этому:

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

Если s равно "", следует вызвать foo. Если строка имеет значение null, что никогда не должно происходить, тогда NullReferenceException вполне подходит (поскольку это, в конце концов, исключительная ситуация).

CodeAnalysis говорит мне проверить s.IsNullOrEmpty. Это изменило бы функциональность непреднамеренно.

Производительность не проблема.

Безопасно ли подавлять соответствующее предупреждение CA1820?

Редактировать: Обновлен пример кода и текст, чтобы лучше отражать мой случай.

Редактировать: Это (слегка измененный) фактический код (в стандартной реализации IXmlSerializable):

public void ReadXml (XmlReader reader)
    // ...
    string img = reader.ReadElementString ("Image");
    if (img != "") {
        Image = Image.FromFile(img);
    }
    // ...

Ответы [ 8 ]

4 голосов
/ 02 ноября 2009

Он будет вести себя иначе по отношению к нулям, так что это зависит от того, что вы хотите, чтобы произошло; Вы упоминаете, что NullReferenceException будет в порядке, но в приведенном коде нет ничего, что могло бы вызвать это, и поэтому это может привести к непредвиденным ошибкам вниз по течению.

У меня никогда нет , но мне всегда хочется добавить:

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

так что я могу использовать:

if (s.IsNullOrEmpty()) foo();
3 голосов
/ 02 ноября 2009

Характеристики:

Если s равно "", следует вызвать foo. Если строка равна нулю, который никогда не должен случиться, тогда NullReferenceException в порядке.

Просто протестируйте длину строки , как указано в правиле CodeAnalysis :

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

Ваш вопрос:

Безопасно ли подавлять связанные CA1820 предупреждение?

Вы можете игнорировать это, ваш код будет работать, но я бы не советовал, следуйте инструкциям столько, сколько сможете. Даже если тема (производительность) не является проблемой, ваш код будет более последовательным, и вы привыкнете писать стандартный код.

2 голосов
/ 02 ноября 2009

С каждым предупреждением анализа кода связана соответствующая документация, доступ к которой можно получить, выделив предупреждение и нажав F1 . Вы также можете щелкнуть правой кнопкой мыши на элементе, чтобы получить помощь.

В любом случае, вот документация , которая объясняет это конкретное предупреждение .

Согласно этой документации, "безопасно исключить предупреждение из этого правила, если производительность не является проблемой".

1 голос
/ 02 ноября 2009

Вы на самом деле не игнорируете предупреждение, вы посмотрели код и решили, что предупреждение не применяется. Это вполне разумное условие для подавления предупреждения.

Чистая спекуляция

Хотел бы я знать немного больше о том, что ты пытался сделать. Я подозреваю, что может быть лучший способ справиться с этим. Шаблон напоминает мне о возвращении сообщения об ошибке или пустом, чтобы сигнализировать об успехе метода. Если это так, я бы рассмотрел либо возврат void и выдачу исключения в случае сбоя, либо возврат bool, и выдачу исключений только в том случае, если сообщение является критическим, и возвращение true / false в противном случае.

1 голос
/ 02 ноября 2009

Было бы лучше написать тест как:

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

Затем вы можете обработать нулевое значение в другом операторе if

0 голосов
/ 02 ноября 2009

Не обрабатывать исключение, в то время как вы можете вообще плохая идея, поэтому CA прав в том, что вам нужно либо рассматривать пустой как пустое, либо обрабатывать исключение Исключение с нулевой ссылкой, вызванное использованием возвращаемого значения, очень плохо. По крайней мере, поместите в Debug.Assert (s! = Null) и сравните со строкой. Пустой

0 голосов
/ 02 ноября 2009

да.

Но я бы согласился с CodeAnalysis со строкой. IsnullOrEmpty - безопасный выбор.

0 голосов
/ 02 ноября 2009

Если ноль в порядке, все будет в порядке в любом случае.

...