Разделить вычисление возвращаемого значения и оператора возврата в однострочных методах?

26

У меня была беседа с коллегой о нарушении returnоператора и оператора, который вычисляет возвращаемое значение в две строки.

Например

private string GetFormattedValue()
{
    var formattedString = format != null ? string.Format(format, value) : value.ToString();
    return formattedString;
}

вместо того

private string GetFormattedValue()
{
    return format != null ? string.Format(format, value) : value.ToString();
}

В коде я не вижу значения в первом варианте. Для меня последнее яснее, особенно для таких коротких методов. Его аргументом было то, что первый вариант легче отлаживать - что довольно мало, поскольку VisualStudio позволяет нам очень детально проверить операторы, когда выполнение остановлено из-за точки останова.

У меня такой вопрос: если по-прежнему уместно писать менее понятный код, просто чтобы облегчить отладку? Есть ли еще какие-либо аргументы для варианта с расщепленным вычислением и returnутверждением?

Пол Керчер
источник
18
Не работает с VS, но предполагает, что вы не можете установить условную точку останова для сложного выражения (или было бы сложно ввести), поэтому, вероятно, поместите assign и return в отдельные операторы, просто для удобства. Компилятор, скорее всего, все равно придумал бы один и тот же код для обоих.
tofro
1
Возможно, это зависит от языка, особенно в языках, где переменные имеют (возможно, сложное) поведение объекта вместо поведения указателя. Утверждение @Paul K, вероятно, верно для языков с поведением указателя, языков, где объекты имеют простое поведение значения, и языков со зрелыми, высококачественными компиляторами.
MSalters
4
«поскольку VisualStudio позволяет нам очень детально проверять операторы, когда выполнение останавливается из-за точки останова», - так оно и есть. Так как же получить возвращаемое значение, если функция вернула структуру с более чем одним членом? (и поддержка этой функции в лучшем случае является пятнистой, существует множество комбинаций, в которых вы не получите возвращаемого значения вообще).
Во
2
Каким образом «очень детальная проверка утверждений» в отладчике, который кто-то использует сегодня, делает плохой вариант написания кода, чтобы его можно было легко отлаживать в ЛЮБОМ отладчике?
Ян
16
Раздражать его еще больше, уменьшая все тело функции доprivate string GetFormattedValue() => string.Format(format ?? "{0}", value);
Грэм

Ответы:

46

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

  • дополнительная переменная не «объясняет» ничего, что не понятно из имени окружающего метода
  • утверждение становится еще длиннее, поэтому (немного) менее читабельным

Более того, в более новых версиях отладчика Visual Studio в большинстве случаев может быть показано возвращаемое значение функции без введения лишней переменной (но будьте осторожны, есть некоторые предостережения, посмотрите на этот старый пост SO и разные ответы ).

Так что в этом конкретном случае я согласен с вами, однако есть и другие случаи, когда объясняющая переменная действительно может улучшить качество кода.

Док Браун
источник
Я также согласен с тем, что, безусловно, есть случаи, когда это полезно.
Пол Керчер
2
Я обычно использую resultв качестве имени переменной. Не намного дольше и проще в отладке
edc65
26
@ edc65: общее имя, подобное « result часто», просто добавляет шум в код и редко повышает читабельность, что и является точкой моего ответа. Это может быть оправдано в контексте отладки, но я бы избегал использования отладчика, которому не нужна отдельная переменная.
Док Браун
6
@JonHanna способ инструмента долго по мне. Имя resultпередает информацию о том, что это значение является результатом функции, так что вы можете посмотреть на нее до того, как функция вернется.
edc65
1
@ edc65, но это делает его полезным. Так что теперь, когда я читаю ваш код, я не сразу понимаю, что это не так. Таким образом, ваш код стал менее читабельным.
Джон Ханна
38

Учитывая факты, которые:

а) На окончательный код не влияют, так как компилятор оптимизирует переменную.

б) Наличие этого отдельно увеличивает возможности отладки.

Я лично пришел к выводу, что это хорошая практика - разделять их в 99% случаев.

Для этого нет никаких материальных недостатков. Аргумент, что он раздувает код, является неправильным, потому что раздутый код является тривиальной проблемой по сравнению с нечитаемым или трудным для отладки кодом. Кроме того, этот метод сам по себе не может создать запутанный код, это полностью зависит от разработчика.

Дом
источник
9
Это правильный ответ для меня. Это облегчает установку точки останова и просмотр значения при отладке, и у меня нет недостатков, о которых я знаю.
Мэтью Джеймс Бриггс
Для точки b в коде Visual Studio просто поместите точку останова в возвращаемое значение, а затем добавьте выражение: GetFormattedValue (), которое покажет результат при достижении точки останова, поэтому дополнительная строка не нужна. Но легче увидеть локальных пользователей с дополнительной строкой, поскольку это не потребует добавления каких-либо дополнительных выражений в отладчике. Так что, действительно, вопрос личных предпочтений.
Джон Рейнор
3
@JonRaynor для возвращаемого значения, поставьте точку останова на закрывающей скобке функции. Он ловит возвращаемое значение, даже в функциях с несколькими возвратами.
Baldrickk
16

Часто введение переменной просто для именования какого-либо результата очень полезно, когда оно делает код более самодокументированным. В данном случае это не имеет значения, поскольку имя переменной очень похоже на имя метода.

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

Но в целом эти решения сильно зависят от ваших личных предпочтений. Например, я нахожу оба решения запутанными, потому что условный оператор используется излишне. Я бы предпочел выражение if. Но в вашей команде вы могли договориться о различных соглашениях. Затем делайте все, что предлагают ваши соглашения. Если соглашения о таком случае молчат, обратите внимание, что это крайне незначительное изменение, которое не имеет значения в долгосрочной перспективе. Если этот шаблон повторяется, возможно, начните обсуждение того, как вы, как команда, хотите справиться с этими случаями. Но это расщепляет волоски между «хорошим кодом» и «возможно, чуть-чуть лучшим кодом».

Амон
источник
1
«Я нахожу оба решения запутанными, потому что условный оператор используется излишне». - Это не реальный пример, я просто должен был что-то придумать, быстро. По общему признанию это не могло бы быть лучшим примером.
Пол Керчер
4
+1 за то, что по сути говорят, что это разница «под радаром», что (при прочих равных условиях) не стоит беспокоиться.
TripeHound
3
@Mindwin, когда я использую троичный оператор, я обычно делю его на несколько строк, чтобы было понятно, что является истинным случаем, а что ложным.
Артуро Торрес Санчес
2
@ ArturoTorresSánchez Я тоже так делаю, но вместо a ?и a :я использую if() {и } else {- - - - \\ :)
Mindwin
3
@Mindwin, но я не могу этого сделать, когда я нахожусь в середине выражения (как инициализатор объекта)
Артуро Торрес Санчес
2

В ответ на ваши вопросы:

У меня такой вопрос: если по-прежнему уместно писать менее понятный код, просто чтобы облегчить отладку?

Да. На самом деле, часть вашего более раннего утверждения кажется мне (без обид) немного недальновидной (см. Жирный шрифт ниже). « Его аргументом было то, что первый вариант легче отлаживать - что довольно мало , поскольку VisualStudio позволяет нам очень детально проверить операторы, когда выполнение остановлено из-за точки останова ».

Упрощение отладки (почти) никогда не бывает « маленькой заслугой », поскольку, по некоторым оценкам, 50% времени программиста тратится на отладку ( Reversible Debugging Software ).

Есть ли еще какие-либо аргументы для варианта с вычислением разделения и оператором возврата?

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

ПРИМЕЧАНИЕ. Бизнес-правила могут лучше обслуживаться в базе данных, поскольку они могут часто меняться. Тем не менее четкое кодирование в этой области все еще имеет первостепенное значение. ( Как создать движок бизнес-правил )

tale852150
источник
1

Я бы пошел еще дальше:

private string GetFormattedValue()
{
    if (format != null) {
        formattedString = string.Format(format, value);
    } else {
        formattedString = value.ToString()
    }
    return formattedString;
}

Зачем?

Использование тернарных операторов для более сложной логики было бы нечитаемым, поэтому вы бы использовали стиль, подобный приведенному выше, для более сложных операторов. Всегда используя этот стиль, ваш код будет последовательным и более простым для анализа человеком. Кроме того, вводя этот вид согласованности (и используя кодовые строки и другие тесты), вы можете избежать goto failошибок типа.

Еще одним преимуществом является то, что ваш отчет о покрытии кода сообщит вам, если вы забыли включить тест для formatне является нулевым. Это не будет иметь место для троичного оператора.


Моя предпочтительная альтернатива - если вы «получаете возврат как можно быстрее», а не против многократного возврата из метода:

private string GetFormattedValue()
{
    if (format != null) {
        return string.Format(format, value);
    }

    return value.ToString();
}

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

Однако важно быть последовательным - и все ваши методы должны следовать одному или другому соглашению.

HorusKol
источник
1
Первый пример кажется плохой практикой, потому что он value.ToString()вызывается без необходимости, когда формат не нулевой. В общем случае это может включать нетривиальные вычисления и может занять больше времени, чем версия, включающая строку формата. Рассмотрим, например, a, в valueкотором PI хранится до миллиона знаков после запятой, и строку формата, которая запрашивает только первые несколько цифр.
Стив
1
почему не то private string GetFormattedValue() => string.Format(format ?? "{0}", value); же самое влияет, и использовать модульные тесты для обеспечения правильности, а не полагаться на отладчик.
Берин Лорич
1
Хотя я согласен, что троичный может быть менее ясным, нулевой терминатор может сделать вещи более ясными. По крайней мере, в этом случае.
Берин Лорич
1
Дорогой дневник, сегодня я прочитал, что написание ясного и краткого кода с использованием хорошо известных (существующих уже около 40 лет) парадигм, идиом и операторов - это кавычка, двойная кавычка, являющаяся умной двойной кавычкой, кавычка - но вместо этого написание чрезмерно многословного кода, нарушающего СУХОЙ и не использовать вышеупомянутые операторы, идиомы и парадигмы, вместо этого пытаясь избежать всего, что может показаться загадочным только пятилетнему ребенку без предшествующего опыта программирования - это ясность вместо этого. Черт, я, должно быть, очень постарел, мой дорогой дневник ... Я должен был научиться идти, когда у меня был шанс.
vaxquis
1
«Использование троичных операторов для более сложной логики было бы нечитабельным». Хотя это действительно так (а я видел людей, которые слишком усложняют логику), это не относится к коду OP, а также не относится к троичным операторам. Единственное, что я могу сказать с полной уверенностью, это то, что линия слишком длинная. gist.github.com/milleniumbug/cf9b62cac32a07899378feef6c06c776 - это то, как я бы его переформатировал.
milleniumbug
1

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

Люди также недооценивают время, которое они тратят на отладку. Они недооценивают, сколько времени они могут потратить, преследуя длинную ошибку. С тестированием я сразу понял, когда добавил ошибку. Это позволяет мне немедленно исправить ошибку, прежде чем она сможет отползти и спрятаться. Есть несколько вещей, более расстраивающих или теряющих время, чем отладка. Не было бы чертовски быстро, если бы мы просто не создавали ошибки в первую очередь?

Западло
источник
Мартин Фаулер - умный человек, и мне понравилось читать его (и ваши) взгляды. Хотя я твердо убежден в том, что тестирование необходимо и что на это нужно потратить больше времени, тот факт, что мы все подвержены ошибкам, говорит о том, что никакие испытания не устранят все ошибки. Следовательно, отладка всегда будет частью процесса разработки и поддержки программы.
tale852150
1

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

Относительно фокуса вашего вопроса, перемещая возвращаемое утверждение, чтобы на него ссылалась переменная ...

Этот вопрос делает 2 предположения, с которыми я не согласен:

  1. Что второй вариант более понятен или легок для чтения (я говорю, что все наоборот), и

  2. что все используют Visual Studio. Я использовал Visual Studio много раз и могу использовать его просто отлично, но обычно я использую что-то еще. Я бы скептически отнесся к среде разработки, которая вызывает конкретную среду IDE.

Выделение чего-либо в именованную переменную редко делает что-либо более сложным для чтения, почти всегда происходит наоборот. Конкретный способ, которым кто-то делает это, может вызвать проблемы, например, если повелитель самодокументирования делает это, var thisVariableIsTheFormattedResultAndWillBeTheReturnValue = ...то, очевидно, это плохо, но это отдельная проблема. var formattedText = ...Это хорошо.

В этом конкретном случае, и, возможно, во многих случаях, так как мы говорим о 1-строчных буквах, переменная не будет вам много говорить о том, что имя функции еще не говорит вам. Следовательно, переменная не добавляет столько. Аргумент отладки все еще может сохраняться, но, опять же, в этом конкретном случае я не вижу ничего, что может быть вашим фокусом при отладке, и его всегда можно легко изменить позже, если кому-то понадобится этот формат для отладки или что-то еще.

В общем, и вы спросили об общем правиле (ваш пример был просто примером обобщенной формы), все замечания, сделанные в пользу варианта 1 (2-строчный), верны. Это хорошие рекомендации. Но руководящие принципы должны быть гибкими. Например, проект, над которым я сейчас работаю, имеет максимум 80 символов на строку, поэтому я разбил много строк, но я обычно нахожу строки с 81-85 символами, которые было бы неудобно разделять или уменьшать читабельность, и я оставляю их более Лимит.

Поскольку это вряд ли добавит ценность, я бы не стал делать 2 строки для конкретного приведенного примера. Я бы сделал вариант 2 (1-линейный), потому что точки не достаточно сильны, чтобы сделать иначе в этом случае.

Аарон
источник