Подтвердить что ты не робот

Кодовые предложения от Resharper делают код менее читаемым?

При попытке добраться до всех зеленых я получил следующее предложение от Resharper.

Исходный код:

    static public string ToNonNullString(this XmlAttribute attr)
    {
        if (attr != null)
            return attr.Value;
        else
            return string.Empty;
    }

Предложение: удалить избыточное 'else', что приведет к следующему:

    static public string ToNonNullString(this XmlAttribute attr)
    {
        if (attr != null)
            return attr.Value;
        return string.Empty;
    }

Для меня предлагаемая версия кажется менее читаемой, чем оригинал. Предлагает ли решение Resharper определение хорошего поддерживаемого кода?

4b9b3361

ОТВЕТЫ

Ответ 1

Технически Resharper верен в том, что "else" не нужно, я предпочитаю прежнюю версию, хотя, поскольку намерение более очевидно.

Сказав это, я бы предпочел:

return attr != null ? attr.Value : string.Empty;

Ответ 2

А, кодовая эстетика. Святое военное время. (Уток)

Я бы пошел либо с выражением::

return attr != null ? attr.Value : String.Empty

или инвертировать if и удалить разрыв строки для создания оговорки охраны:

if (attr == null) return String.Empty;

return attr.Value;

Ответ 3

Я думаю, что новая версия намного лучше, если вы инвертируете if

static public string ToNonNullString(this XmlAttribute attr)
{
    if (attr == null)
        return string.Empty;

    return attr.Value;
}

Потому что ваша оригинальная версия слишком симметрична, а нуль-случай - это особый случай.

Новая версия более читаема в терминах "что она возвращает большую часть времени?".

Ответ 4

Я согласен с тем, что первая версия вашего кода более читаема.

Я нашел предложения Resharper в этих случаях не всегда полезными, хотя иногда он может очищать вещи. Вот почему я настроил Resharper, чтобы показать изменение как "подсказку", а не "предложение". Это приведет к тому, что зеленое подчеркивание будет менее видимым, и оно не будет выделено на правой боковой панели.

Ответ 5

Если вам не нравится, как ReSharper предлагает что-то, просто отключить конкретное предложение (подсказка косой черты). То же самое касается стиля кодирования, который, как мне кажется, довольно с высокой степенью конфигурирования. Утверждение ReSharper непригодным для использования (цитирование "Я рад сказать, что оно не сохранилось, никто здесь больше не использует его" ) только потому, что вы не занимаете 5 минут, чтобы узнать, как настроить его, просто plain stupid.

Конечно, вы не должны позволять некоторому инструменту диктовать какую-то часть вашего стиля кодирования, а ReSharper НЕ делает этого, если вы не говорите ему об этом. Это просто.

Ответ 6

Ваш исходный код намного читабельнее и понятен - с первого взгляда вы можете точно увидеть условие, которое возвращает string.Empty. Без else вы должны увидеть, что перед этим есть возврат в if.

Просто помни, ты человек и по своей природе умнее машины. Если он говорит вам, что что-то лучше, и вы не согласны, тогда не слушайте его.

Ответ 7

Мой стандарт кодирования всегда использует скобки (даже если есть только одна команда после команды)
Это требует немного усилий (больше набрав), но я так часто убеждаюсь, что это того стоит!

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

Мне нравится то, что предложил Решарпер. Особенно при наличии вложенных операторов if:

Предположим, что у нас есть этот код:

   if (condition1)  {
      instruction1;
   }
   else {
       if (condition2) {
           instruction2;
       }
   }

Его можно изменить так, чтобы он выглядел следующим образом:

   if (condition1)  {
      instruction1;
   }       
   else if (condition2) {
      instruction2;
   }

И это гораздо более читаемо для меня, чем раньше.
(Это было бы также более заметно, если у вас более 2-х уровневых вложенных операторов)

Ответ 8

Я заметил то же самое в ReSharper, поэтому я ценю его способность отключать некоторые элементы или понижать их уровень предупреждения. Я также озадачен этим предложением:

SomeClass varName = new SomeClass();

имеет предлагаемое изменение:

var varName = new SomeClass();

Да, я знаю, что мне не нужен начальный тип объявления, но кажется странным предположить, что форма var как-то лучше, чем другая. Кто-нибудь знаком с обоснованием предложения или согласен со мной, что это тоже странно?

Ответ 9

Классический пример исключений, которые ползут во всем, когда вы используете небольшой размер выборки. Рефакторинг огромного блока if-elseif-else в макет предложения охраны делает код более читабельным, но, как вы говорите, если вы применяете одни и те же правила к одному if-else, это не так полезно. Я бы зашел так далеко, чтобы сказать, что это (легкое) отсутствие предвидения со стороны разработчиков-разработчиков, чтобы не пропустить очень маленькие блоки, такие как это, но это было достаточно безвредно.

Ответ 10

Являясь noob на С# и больше используемым для C и Java, я до сих пор не могу привыкнуть к размещению угловых скобок в С#.NET и VS. Отложив все это, я согласен с Андреем в том, что инвертирование "if" еще более удобочитаемо. С другой стороны, я лично считаю, что исключение "else" уменьшает читаемость (слегка). Я бы пошел с этим лично:

static public string ToNonNullString(this XmlAttribute attr)
{    
    if (attr == null)
        return string.Empty;
    else
        return attr.Value;
}

Ответ 11

Единственное, что я добавлю, должно быть связано с длиной задействованных выражений. Лично мне нравится компактность тернарных выражений, но что-то вроде

if (testDateTime > BaseDateTime)
    return item.TransactionDate <= testDateTime && item.TransactionDate >= BaseDateTime;

return item.TransactionDate >= testDateTime && item.TransactionDate <= BaseDateTime;

во что-то вроде

return testDateTime > BaseDateTime ? item.TransactionDate <= testDateTime && item.TransactionDate >= BaseDateTime : item.TransactionDate >= testDateTime && item.TransactionDate <= BaseDateTime;

для меня действительно не помогает.

Ответ 12

Его всегда спорно, когда речь идет о передовой практике и стандартам кодирования. Одной из причин этого является то, что они не могут быть легко реализованы с использованием среды IDE, такой как Visual Studio. Существуют инструменты, доступные как FxCop и StyleCop, которые можно использовать для анализа кода для стандартов. FxCop используется для анализа скомпилированного кода, а StyleCop используется для анализа исходного кода.

Вы можете настроить StyleCop на минутный уровень в отношении того, какое форматирование вы хотите применить к коду. Существует надстройка под названием StyleCop for Resharper, которая дает предложения прямо внутри Visual Studio. У меня было подробное сообщение в блоге о том же в http://nileshgule.blogspot.com/2010/10/refactoring-clean-code-using-resharper.html

Ответ 13

Вариант resharper лучше, так как условие "attr!= null" можно рассматривать как ранний путь спасения (или путь исключения в случае использования), позволяя функции продолжать свою основную задачу. (ни у меня не выигрывают высокие пятеро, я ненавижу несколько возвратов).

В этом случае я бы сказал, что единственная линейка one-liner от MrWiggles - лучшая альтернатива.

Ответ 14

Некоторые мои коллеги начали использовать Resharper с этого момента на страницах, которые они редактировали, были ужасны в макете и удобочитаемости. Я рад сказать, что он не выжил, никто больше его не использует.

Об этом заявлении я согласен с Джеффри Хэнтином, inline-if, очень хорошо подходит для такого типа утверждения, и решение Whatsit очень доступно. За некоторыми исключениями я (personaly) говорят, что методы/функции должны иметь только 1 оператор return.

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

Оба замечания должны использоваться как "думать о" не как правило, так же как и большинство подобных проблем, всегда используйте свой мозг:) большинство ошибок возникает, когда вы этого не делаете.

В заключение: Скажи "НЕТ", чтобы повторить! (да, я действительно не люблю Решарпера, извините.)