Лучшая практика по если / возврат

60

Я хочу знать, что считается лучшим способом возвращения, когда у меня есть ifзаявление.

Пример 1:

public bool MyFunction()
{
   // Get some string for this example
   string myString = GetString();

   if (myString == null)
   {
      return false;
   }
   else
   {
      myString = "Name " + myString;
      // Do something more here...
      return true;
   }
}

Пример 2:

public bool MyFunction()
{
   // Get some string for this example
   string myString = GetString();

   if (myString == null)
   {
      return false;
   }

   myString = "Name " + myString;
   // Do something more here...
   return true;
}

Как вы можете видеть в обоих примерах, функция вернется, true/falseно стоит ли ставить elseоператор так, как в первом примере, или лучше его не ставить?

СЭД
источник
7
Если вы проверяете только ошибки в первом «if», лучше не включать «else», потому что ошибки не должны рассматриваться как часть реальной логики.
Мерт Акчакая
2
лично я бы больше беспокоился о функции, вызывающей побочные эффекты, но я думаю, что это просто плохо выбранный пример?
JK.
14
На мой взгляд , первый вариант, как отвергая все студенты в комнате , кто не закончил свою домашнюю работу, а затем , как только они ушли, сказав остальным студентам «Теперь , если вы даже закончить свою домашнюю работу ...». Это имеет смысл, но не нужно. Поскольку условное больше не является условным, я склонен отбрасывать else.
Николь
1
Что «Делай что-то еще здесь», что вы хотите сделать? Это может полностью изменить ваш дизайн.
Бенедикт

Ответы:

81

Пример 2 известен как защитный блок. Лучше всего возвращать / выдавать исключение раньше, если что-то пошло не так (неправильный параметр или недопустимое состояние). В нормальном логическом потоке лучше использовать Пример 1

Kemoda
источник
+1 - хорошее различие, и на что я собирался ответить.
Теластин
3
@ pR0Ps имеет тот же ответ ниже и предоставляет пример кода, объясняющий, почему он в итоге стал более чистым путем для подражания. +1 за хороший ответ.
Этот вопрос задавался много раз на SO, и хотя он может быть спорным, это хороший ответ. У многих людей, которые были обучены возвращаться только с конца, есть некоторые проблемы с этой концепцией.
Билл К
2
+1. Неправильное избегание защитных блоков приводит к появлению кода стрелки.
Брайан
Разве оба примера не показывают блок защиты?
Эрни
44

Мой личный стиль - использовать сингл ifдля защитных блоков и if/ elseв реальном коде обработки метода.

В этом случае вы используете в myString == nullкачестве защитного условия, поэтому я бы предпочел использовать один ifшаблон.

Рассмотрим код, который немного сложнее:

Пример 1:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }
    else{
        //processing block
        myString = escapedString(myString);

        if (myString == "foo"){
            //some processing here
            return false;
        }
        else{
            myString = "Name " + myString;
            //other stuff
            return true;
        }
    }
}

Пример 2:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }

    //processing block
    myString = escapedString(myString);

    if (myString == "foo"){
        //some processing here
        return false;
    }
    else{
        myString = "Name " + myString;
        //other stuff
        return true;
    }
}

В примере 1 и охранник, и остальная часть метода находятся в форме if/ else. Сравните это с Примером 2, где защитный блок находится в единственной ifформе, тогда как остальная часть метода использует форму if/ else. Лично я считаю, что пример 2 легче понять, в то время как пример 1 выглядит неопрятно и с чрезмерным отступом.

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

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

оборота pR0Ps
источник
18
Пример 2 будет еще легче прочитать, если вы избавитесь от второго.
briddums
18

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

GSto
источник
+1 за меньшее отступ. Этот жест очевиден, если у вас есть несколько проверок
д.раев
15

Моя личная практика заключается в следующем:

  • Мне не нравятся функции с несколькими точками выхода, мне было трудно поддерживать и следить, модификации кода иногда нарушают внутреннюю логику, потому что она по своей природе немного небрежна. Когда это сложный расчет, я создаю возвращаемое значение в начале и возвращаю его в конце. Это заставляет меня тщательно следовать каждому пути if-else, switch и т. Д., Правильно устанавливать значение в нужных местах. Я также трачу немного времени на решение, устанавливать ли возвращаемое значение по умолчанию или оставить его неинициализированным в начале. Этот метод также помогает при изменении логики или типа или значения возвращаемого значения.

Например:

public bool myFunction()
{
   // First parameter loading
   String myString = getString();

   // Location of "quick exits", see the second example
   // ...

   // declaration of external resources that MUST be released whatever happens
   // ...

   // the return variable (should think about giving it a default value or not) 
   // if you have no default value, declare it final! You will get compiler 
   // error when you try to set it multiple times or leave uninitialized!
   bool didSomething = false;

   try {
     if (myString != null)
     {
       myString = "Name " + myString;
       // Do something more here...

       didSomething = true;
     } else {
       // get other parameters and data
       if ( other conditions apply ) {
         // do something else
         didSomething = true;
       }
     }

     // Edit: previously forgot the most important advantage of this version
     // *** HOUSEKEEPING!!! ***

   } finally {

     // this is the common place to release all resources, reset all state variables

     // Yes, if you use try-finally, you will get here from any internal returns too.
     // As I said, it is only my taste that I like to have one, straightforward path 
     // leading here, and this works even if you don't use the try-finally version.

   }

   return didSomething;
}
  • Единственное исключение: «быстрый выход» при запуске (или в редких случаях внутри процесса). Если реальная логика вычислений не может обработать определенную комбинацию входных параметров и внутренних состояний или имеет простое решение без запуска алгоритма, это не поможет инкапсулировать весь код в (иногда глубокие) блоки if. Это «исключительное состояние», не входящее в основную логику, поэтому я должен выйти из расчета, как только обнаружу. В этом случае ветки нет, в нормальных условиях выполнение просто продолжается. (Конечно, «исключительное состояние» лучше выражать, выбрасывая исключение, но иногда это избыточное убийство.)

Например:

public bool myFunction()
{
   String myString = getString();

   if (null == myString)
   {
     // there is nothing to do if myString is null
     return false;
   } 

   myString = "Name " + myString;
   // Do something more here...

   // not using return value variable now, because the operation is straightforward.
   // if the operation is complex, use the variable as the previous example.

   return true;
}

Правило «одного выхода» также помогает, когда для расчета требуются внешние ресурсы, которые вы должны высвободить, или говорится, что вы должны сбросить их перед выходом из функции; иногда они добавляются позже во время разработки. С несколькими выходами внутри алгоритма намного сложнее правильно расширить все ветви. (И если могут возникать исключения, то сброс / сброс также следует поместить в блок finally, чтобы избежать побочных эффектов в редких исключительных случаях ...).

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

Lorand Kedves
источник
9
+1 за «Мне не нравятся функции с несколькими точками выхода»
Corv1nus
@LorandKedves - добавил пример - надеюсь, вы не возражаете.
Мэтью Флинн
@ MatthewFlynn хорошо, если вы не возражаете, что это противоречит тому, что я предложил в конце моего ответа ;-) Я продолжаю пример, я надеюсь, что в конце концов это будет хорошо для нас обоих :-)
Lorand Kedves
@MatthewFlynn (извините, я просто ублюдок, и спасибо, что помогли мне прояснить мое мнение. Надеюсь, вам понравилась текущая версия.)
Лоран Кедвес
13
Из функций с несколькими точками выхода и функций с изменяемой переменной результата первое кажется мне гораздо меньшим из двух зол.
Джон Перди
9

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

  1. Они позволяют быстрый выход при определенных условиях.
  2. Удалить необходимость сложных и ненужных операторов if позже в коде.

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

оборота дрекка
источник
5

Мне нравится подход "Fall Through":

public bool MyFunction()
{
   string myString = GetString();

   if (myString != null)
   {
     myString = "Name " + myString;
     return true;
    }
    return false;
}

У действия есть определенное условие, все остальное - просто возврат по умолчанию.

Темная ночь
источник
1

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

Picuture это. Предположим, что тесты сложные, и вы действительно не хотите связывать их в одно условие if-ORed, чтобы избежать сложности:

//Style1
if (this1 != Right)
{ 
    return;
}
else if(this2 != right2)
{
    return;
}
else if(this3 != right2)
{
    return;
}
else
{
    //everything is right
    //do something
    return;
}

против

//Style 2
if (this1 != Right)
{ 
   return;
}
if(this2 != right2)
{
    return;
}
if(this3 != right2)
{
    return;
}


//everything is right
//do something
return;

Здесь есть два основных преимущества

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

  2. Если вам нужно добавить / удалить одно условие, вы уменьшите свои шансы испортить всю лестницу if-elseif-else.

Еще одно незначительное преимущество - у вас есть наименьшее количество скобок для ухода.

DPD
источник
0

Это должно быть вопросом, который звучит лучше.

If condition
  do something
else
  do somethingelse

выражается лучше, чем

if condition
  do something
do somethingelse

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

Хосе Валенте
источник
3
Если метод настолько длинный, что трудно понять вторую форму, он слишком длинный.
Кевин Клайн
7
Ваши два случая эквивалентны, только если do somethingвключает в себя возврат. Если нет, второй код будет выполняться do somethingelseнезависимо от ifпредиката, который не работает как первый блок.
Аднан
Это также то, что ОП объяснял в своих примерах. Просто следуя линии вопроса
Хосе Валенте
0

Я нахожу пример 1 раздражающим, потому что пропущенный оператор return в конце функции, возвращающей значение, сразу вызывает флаг «Подождите, что-то не так». Так что в таких случаях я бы пошел с примером 2.

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

Безопасный
источник
0

Когда ваш ifоператор всегда возвращается, нет смысла использовать else для оставшегося кода в функции. Это добавляет дополнительные строки и дополнительные отступы. Добавление ненужного кода затрудняет чтение, и, как всем известно, чтение кода - это сложная задача .

briddums
источник
0

В вашем примере elseэто явно не нужно, НО ...

Быстро просматривая строки кода, глаза обычно смотрят на фигурные скобки и отступы, чтобы получить представление о потоке кода; прежде чем они на самом деле обосноваться на самом коде. Следовательно, написание elseскобок и отступов вокруг второго блока кода позволяет читателю быстрее увидеть, что это ситуация «A или B», когда один или другой блок будет выполняться. То есть читатель видит фигурные скобки и отступы ДО того, как они увидят returnпосле инициала if.

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

Дауд говорит восстановить Монику
источник
0

Я бы предпочел пример 2, потому что сразу видно, что функция возвращает что-то . Но даже более того, я бы предпочел вернуться из одного места, например так:

public bool MyFunction()
{
    bool result = false;

    string myString = GetString();

    if (myString != nil) {
        myString = "Name " + myString;

        result = true;
    }

    return result;
}

С этим стилем я могу:

  1. Смотри сразу, что я что-то возвращаю.

  2. Поймать результат каждого вызова функции только с одной точкой останова.

Калеб
источник
Это похоже на то, как я подхожу к проблеме. Единственное отличие состоит в том, что я использовал бы переменную результата, чтобы получить оценку myString и использовать ее в качестве возвращаемого значения.
Чак Конвей
@ChuckConway Согласен, но я пытался придерживаться прототипа ОП.
Калеб
0

Мой личный стиль имеет тенденцию идти

function doQuery(string) {
    if (!query(string)) {
        query("ABORT");
        return false;
    } // else
    if(!anotherquery(string) {
        query("ABORT");
        return false;
    } // else
    return true;
}

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

Shadur
источник
1
Лично я надеюсь, что мне никогда не придется работать с вашим кодом.
CVn
Если у вас есть несколько проверок, этот метод может быть немного длинным. Возврат из каждого условия if аналогичен использованию оператора goto для пропуска разделов кода.
Чак Конуэй
Если бы это был выбор между этим и кодом с elseвключенными предложениями, я бы выбрал последнее, потому что компилятор также будет эффективно их игнорировать. Хотя это будет зависеть от того, else if не увеличит ли отступ больше, чем за один раз - если бы это произошло, я бы с тобой согласился Но мой выбор - elseполностью отказаться, если этот стиль будет разрешен.
Марк Херд
0

Я бы написал

public bool SetUserId(int id)
{
   // Get user name from id
   string userName = GetNameById(id);

   if (userName != null)
   {
       // update local ID and userName
       _id = id;
       _userNameLabelText = "Name: " + userName;
   }

   return userName != null;
}

Я предпочитаю этот стиль, потому что наиболее очевидно, что я бы вернулся userName != null.

мин
источник
1
Вопрос в том, почему вы предпочитаете этот стиль?
Калеб
... Честно говоря, я не уверен, почему вы вообще хотите использовать строку в этом случае, не говоря уже о том, почему вы добавляете дополнительный логический тест в конце, который вам действительно не нужен по какой-либо причине.
Шадур
@Shadur Я знаю о дополнительном булевом тесте и неиспользованном myStringрезультате. Я просто копирую функцию из OP. Я изменю это на что-то, что выглядит более реальным. Булев тест является тривиальным и не должен быть проблемой.
TIA
1
@Shadur Мы не занимаемся оптимизацией здесь, верно? Моя цель - сделать мой код легким для чтения и сопровождения, и лично я бы заплатил за это одной проверкой нулевой ссылки.
TIA
1
@tia Мне нравится дух твоего кода. Вместо того, чтобы оценивать userName дважды, я записываю первую оценку в переменную и возвращаю ее.
Чак Конуэй
-1

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

Таким образом, я стараюсь не слишком увлекаться своими ветвлениями, поскольку всякий раз, когда я в конечном итоге это делаю, я кодирую логику приложения в свои if, что усложняет их поддержку (например, если enum OK или ERR и я пишу все свои if пользуясь тем фактом, что! OK <-> ERR, в задницу становится больно добавлять третий вариант в перечисление в следующий раз.)

В вашем случае, например, я бы использовал простой if, так как «return if null» обязательно будет распространенным шаблоном, и маловероятно, что вам придется беспокоиться о третьей возможности (в дополнение к null / not null) в будущем.

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

hugomg
источник
-1

Я думаю, что в Примере 2 есть много подводных камней, которые в будущем могут привести к непреднамеренному коду. Во-первых, в основе этого лежит логика, окружающая переменную myString. Поэтому , чтобы быть явным, все испытания условий должно происходить в блоке учета кода для известны и по умолчанию / неизвестной логики.

Что делать, если позже в примере 2 непреднамеренно был введен код, который значительно изменил результат:

   if (myString == null)
   {
      return false;
   }

   //add some v1 update code here...
   myString = "And the winner is: ";
   //add some v2 update code here...
   //buried in v2 updates the following line was added
   myString = null;
   //add some v3 update code here...
   //Well technically this should not be hit because myString = null
   //but we already passed that logic
   myString = "Name " + myString;
   // Do something more here...
   return true;

Я думаю, что с elseблоком, следующим сразу за проверкой на ноль, программисты, которые добавили усовершенствования в будущие версии, добавили бы всю логику вместе, потому что теперь у нас есть строка логики, которая была непреднамеренной для исходного правила (возвращая, если значение равно значение NULL).

Я глубоко верю в это некоторым руководствам C # по Codeplex (ссылка на них здесь: http://csharpguidelines.codeplex.com/ ), в которых говорится следующее:

«Добавьте описательный комментарий, если блок по умолчанию (иное) должен быть пустым. Более того, если этот блок не должен быть достигнут, выведите исключение InvalidOperationException для обнаружения будущих изменений, которые могут попасть в существующие случаи. Это гарантирует лучший код, потому что все пути, по которым может пройти код, были продуманы ".

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

atconway
источник
Как не использование else в примере два не учитывает все случаи? Ожидаемый результат - продолжение исполнения. Добавление предложения else в этом случае только увеличивает сложность метода.
Чак Конуэй
Я не согласен, основываясь на способности не иметь всех заинтересованных логики в кратком и детерминистическом блоке. Основное внимание уделяется манипулированию myStringзначением, а код, следующий за ifблоком, является результирующей логикой, если строка! = Null. Поэтому, поскольку основное внимание уделяется дополнительной логике, управляющей этим конкретным значением, я думаю, что оно должно быть заключено в elseблок. В противном случае это открывает потенциал, как я показал, чтобы непреднамеренно отделить логику и ввести непредвиденные последствия. Может случиться не все время, но это действительно был вопрос стиля, основанного на передовом опыте .
atconway