Использование return внутри метода void - плохая практика?

92

Представьте себе следующий код:

void DoThis()
{
    if (!isValid) return;

    DoThat();
}

void DoThat() {
    Console.WriteLine("DoThat()");
}

Можно ли использовать return внутри метода void? Есть ли снижение производительности? Или лучше написать такой код:

void DoThis()
{
    if (isValid)
    {
        DoThat();
    }
}

источник
1
А как насчет: void DoThis () {if (isValid) DoThat (); }
Dscoduc
30
представьте себе код? Зачем? Это прямо там! :-D
STW
Это хороший вопрос, я всегда думаю, что использовать return - хорошая практика; для выхода из метода или функции. Особенно в методе интеллектуального анализа данных LINQ, имеющем несколько результатов IQueryable <T>, и все они зависят друг от друга. Если один из них не дал результата, предупредите и выйдите.
Cheung

Ответы:

173

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

А меньшая вложенность ваших методов улучшает читаемость кода и удобство сопровождения.

Фактически, если у вас есть метод void без какого-либо оператора return, компилятор всегда будет генерировать инструкцию ret в конце его.

Кристиан К. Сальвадо
источник
33

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

Рассмотреть возможность:

void MyFunc(object obj)
{
    if (obj != null)
    {
        obj.DoSomething();
    }
}

против:

void MyFunc(object obj)
{
    if (obj == null)
        return;

    obj.DoSomething();
}

Теперь представьте, что другой программист добавляет строку: obj.DoSomethingElse ();

void MyFunc(object obj)
{
    if (obj != null)
    {
        obj.DoSomething();
    }

    obj.DoSomethingElse();
}

void MyFunc(object obj)
{
    if (obj == null)
        return;

    obj.DoSomething();
    obj.DoSomethingElse();
}

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

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

Джейсон Уильямс
источник
Да, но с другой стороны, несколько уровней вложенности с их условиями делают код еще более подверженным ошибкам, сложнее отслеживать логику и, что более важно, труднее отлаживать. Плоские функции - меньшее зло, ИМО.
Скрим
18
Я выступаю за сокращение вложенности! :-)
Джейсон Уильямс
Я согласен с этим. Кроме того, с точки зрения рефакторинга, проще и безопаснее реорганизовать метод, если obj становится структурой или что-то, что вы можете гарантировать, не будет иметь значение null.
Фил Купер,
18

Плохая практика ??? Ни за что. Фактически, всегда лучше обрабатывать проверки, возвращаясь из метода как можно раньше, если проверки не пройдут. В противном случае это привело бы к огромному количеству вложенных ifs & elses. Раннее завершение улучшает читаемость кода.

Также проверьте ответы на аналогичный вопрос: следует ли использовать оператор return / continue вместо if-else?

SO Пользователь
источник
8

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

Майк Холл
источник
8

В первом примере используется инструкция защиты. Из Википедии :

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

Я думаю, что наличие группы охранников на вершине метода - вполне понятный способ программирования. По сути, это означает, что «не выполняйте этот метод, если хотя бы один из них верен».

В общем, хотелось бы такого:

void DoThis()
{
  if (guard1) return;
  if (guard2) return;
  ...
  if (guardN) return;

  DoThat();
}

Я думаю, тогда это будет намного удобнее:

void DoThis()
{
  if (guard1 && guard2 && guard3)
  {
    DoThat();
  }
}
cdmckay
источник
3

Нет потери производительности, однако второй фрагмент кода более читабелен и, следовательно, его легче поддерживать.

Рассел
источник
Рассел: Я не согласен с твоим мнением, но тебе не следовало голосовать за это. +1, чтобы выровнять это. Кстати, я считаю, что логический тест и возврат в одной строке, за которой следует пустая строка, являются четким указанием на то, что происходит. например, первый пример Родриго.
Пол Сасик
Я не согласен с этим. Увеличение вложенности не улучшает читаемость. В первом фрагменте кода используется «охранный» оператор, что является вполне понятным шаблоном.
cdmckay
Я тоже не согласен. Предусмотренные меры предосторожности, позволяющие преждевременно отказываться от функции, в наши дни обычно считаются хорошим средством, помогающим читателю понять реализацию.
Пит Ходжсон,
2

В этом случае ваш второй пример - лучший код, но он не имеет ничего общего с возвратом из функции void, это просто потому, что второй код более прямой. Но возврат из функции void - это нормально.

Имажинист
источник
0

Это нормально и без «потери производительности», но никогда не пишите оператор «if» без скобок.

Всегда

if( foo ){
    return;
}

Это более читабельно; и вы никогда случайно не предположите, что некоторые части кода находятся внутри этого оператора, хотя это не так.

Полдень шелк
источник
2
читабельность - это субъективно. imho, все, что добавлено в код, что не нужно, делает его менее читаемым ... (я должен прочитать больше, а потом мне интересно, почему он там, и тратить время, пытаясь убедиться, что я что-то не упускаю) ... но это мой субъективное мнение
Чарльз Бретана
10
Лучшая причина всегда включать фигурные скобки - это не столько удобочитаемость, сколько безопасность. Без фигурных скобок кому-то будет легко позже исправить ошибку, которая требует дополнительных операторов как часть if, не обращайте достаточно внимания и не добавляйте их без добавления фигурных скобок. Если всегда использовать скобы, этот риск устраняется.
Скотт Дорман,
2
Шелковистый, пожалуйста, нажмите Enter перед вашим {. Это совпадает {с вашим }в одном столбце, что значительно облегчает читаемость (гораздо проще найти соответствующие открывающие / закрывающие фигурные скобки).
Imagist
1
@Imagist Я оставлю это на усмотрение; и сделано так, как я предпочитаю :)
Noon Silk
1
Если каждая закрывающая скобка сопоставлена ​​с открытой фигурной скобкой, которая находится на том же уровне отступа , то визуально различить, какие ifоператоры требуют закрывающих фигурных скобок, будет легко, и, таким образом, ifуправление операторами одним оператором будет безопасным. Перемещение открытой фигурной скобки обратно к строке с помощью ifсохраняет строку вертикального пространства в каждом мульти-операторе if, но потребует использования строки закрывающей скобки, которая в противном случае была бы ненужной.
supercat
0

Я собираюсь не согласиться со всеми вами, молодые хищники, по этому поводу.

Использование return в середине метода, недействительного или нет, является очень плохой практикой по причинам, которые были довольно четко сформулированы почти сорок лет назад покойным Эдсгером В. Дейкстра, начиная с хорошо известного «Заявление GOTO, которое считается вредным. ", и продолжается в" Структурированном программировании "Даля, Дейкстры и Хора.

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

«Заявление GOTO считается вредным» и «Структурированное программирование» положили начало революции «структурированного программирования» 1970-х годов. Эти две части являются причиной того, что у нас сегодня есть if-then-else, while-do и другие явные управляющие конструкции, и почему операторы GOTO в языках высокого уровня находятся в списке исчезающих видов. (Мое личное мнение, что они должны быть в списке вымерших видов.)

Стоит отметить, что Модулятор потока сообщений, первая часть военного программного обеспечения, которая КОГДА-ЛИБО прошла приемочные испытания с первой попытки, без отклонений, отказов или словоблудия «да, но», была написана на языке, который даже не имел заявление GOTO.

Также стоит упомянуть, что Никлаус Вирт изменил семантику оператора RETURN в Oberon-07, последней версии языка программирования Oberon, сделав его конечной частью объявления типизированной процедуры (т. Е. Функции), а не исполняемый оператор в теле функции. В его объяснении изменения говорилось, что он сделал это именно потому, что предыдущая форма БЫЛА нарушением принципа одного выхода структурного программирования.

Джон Р. Стром
источник
2
@John: мы преодолели запрет Dykstra о множественных возвратах примерно в то время, когда мы перешли через Паскаль (по крайней мере, большинство из нас).
Джон Сондерс,
Случаи, когда требуется несколько возвратов, часто являются признаком того, что метод пытается сделать слишком много и его следует сократить. Я не собираюсь заходить в этом так далеко, как Джон, и оператор return как часть проверки параметров может быть разумным исключением, но я понимаю, откуда взялась идея.
kyoryu
@nairdaen: В этом квартале до сих пор ведутся споры по поводу исключений. Мой совет таков: если разрабатываемая система ДОЛЖНА исправить проблему, которая вызвала исходное исключительное состояние, и я не против разозлить парня, которому придется писать этот код, я выброшу исключение. Затем на собрании на меня кричат, потому что парень не потрудился поймать исключение, и приложение вылетело при тестировании, и я объясняю, ПОЧЕМУ он должен исправить проблему, и все снова улаживается.
Джон Р. Стром
Есть большая разница между операторами охраны и gotos. Зло gotos в том, что они могут прыгать куда угодно, поэтому распутывать и запоминать может быть очень сложно. Операторы защиты - полная противоположность - они обеспечивают закрытый вход в метод, после которого вы знаете, что работаете в «безопасной» среде, уменьшая количество вещей, которые вы должны учитывать при написании остальной части кода (например, «Я знаю, что этот указатель никогда не будет нулевым, поэтому мне не нужно обрабатывать этот случай во всем коде»).
Джейсон Уильямс,
@Jason: Изначально вопрос касался не операторов защиты, а операторов случайного возврата в середине метода. Приведенный пример выглядит как охранник. Ключевая проблема заключается в том, что на сайте возврата вы хотите иметь возможность рассуждать о том, что метод сделал или не сделал, а случайные возвраты усложняют это ТОЧНО по тем же причинам, что и случайные GOTO. См .: Дейкстра, «Заявление GOTO считается вредным». Что касается синтаксиса, cdmckay в другом ответе дал свой предпочтительный синтаксис для охранников; Я не согласен с его мнением о том, какая форма более читабельна.
Джон Р. Стром
0

При использовании охранников обязательно следуйте определенным правилам, чтобы не запутать читателей.

  • функция делает одно
  • охранники представлены только как первые логика в функции
  • безгнездная часть содержит функцию в основном замысле

пример

// guards point you to the core intent
void Remove(RayCastResult rayHit){

  if(rayHit== RayCastResult.Empty)
    return
    ;
  rayHit.Collider.Parent.Remove();
}

// no guards needed: function split into multiple cases
int WonOrLostMoney(int flaw)=>
  flaw==0 ? 100 :
  flaw<10 ? 30 :
  flaw<20 ? 0 :
  -20
;
Орангут
источник
-3

Выбрасывать исключение вместо того, чтобы ничего не возвращать, когда объект равен нулю и т. Д.

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

Но в остальном досрочный возврат - неплохая практика.

Дхананджай
источник
1
Ответ не отвечает на вопрос. Вопрос в методе void, поэтому ничего не возвращается. Кроме того, у методов нет параметров. Я понимаю, что не возвращаю null, если тип возвращаемого значения является объектом, но это не относится к этому вопросу.
Люк Хаммер