Считаете ли вы эту технику "ПЛОХОЙ"? - PullRequest
20 голосов
/ 28 октября 2008

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

Так что я делаю трюк так:

do
{
   bool isGood = true;

   .... some code

   if(!isGood)
       break;

   .... some more code

   if(!isGood)
       break;

   .... some more code

 } while(false);

 ..... some other code, which has to be executed.

Я использую «фальшивый» цикл, который запускается один раз, и я могу прервать его, набрав break или continue .

Некоторым моим коллегам это не понравилось, и они назвали это «плохой практикой». Лично я нахожу такой подход довольно гладким. Но что ты думаешь?

Ответы [ 28 ]

45 голосов
/ 28 октября 2008

Плохая практика, это зависит.

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

Существует несколько альтернатив этому коду, которые могут быть или не быть лучше, в зависимости от ситуации.

Ваше решение "сделать / пока"

Ваше решение интересно, если у вас много кода, но в некоторых ограниченных точках будет оцениваться «выход» этой обработки:

do
{
   bool isError = false ;

   /* some code, perhaps setting isError to true */
   if(isError) break ;
   /* some code, perhaps setting isError to true */
   if(isError) break ;
   /* some code, perhaps setting isError to true */
}
while(false) ;

// some other code   

Проблема в том, что вы не можете легко использовать свой "if (isError) break;" является циклом, потому что он будет выходить только из внутреннего цикла, а не из блока do / while.

И, конечно, если сбой происходит внутри другой функции, функция должна возвращать какой-то код ошибки, и ваш код не должен забывать правильно интерпретировать код ошибки.

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

Вызов goto a ... goto

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

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

Вы все равно должны открыть блок и вместо взлома использовать goto.

{
   // etc.
   if(/*some failure condition*/) goto MY_EXIT ;
   // etc.

   while(/* etc.*/)
   {
      // etc.
      for(/* etc.*/)
      {
         // etc.
         if(/*some failure condition*/) goto MY_EXIT ;
         // etc.
      }
      // etc.
      if(/*some failure condition*/) goto MY_EXIT ;
      // etc.
   }

   // etc.
}

MY_EXIT:

// some other code   

Таким образом, когда вы выходите из блока через goto, вы не можете обойти какой-либо конструктор объекта с помощью goto (что запрещено C ++).

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

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

попытаться / поймать

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

try
{
   // All your code
   // You can throw the moment something fails
   // Note that you can call functions, use reccursion,
   // have multiple loops, etc. it won't change
   // anything: If you want to exit the process,
   // then throw a MyExitProcessException exception.

   if(/* etc. */)
   {
      // etc.
      while(/* etc.*/)
      {
         // etc.
         for(/* etc.*/)
         {
            // etc.
            if(/*some failure condition*/) throw MyExitProcessException() ;
            // etc.
         }
         // etc.

         callSomeFunction() ;
         // the function will throw if the condition is met
         // so no need to test a return code

         // etc.
      }
      // etc.
   }

   // etc.
}
catch(const MyExitProcessException & e)
{
   // To avoid catching other exceptions, you should
   // define a "MyExitProcessException" exception
}

// some other code

Если какое-либо условие в коде выше или внутри некоторых функций, вызываемых кодом выше, не выполняется, то генерируется исключение.

Это несколько весомее, чем ваше решение do / while, но имеет те же преимущества и может даже прервать обработку из внутренних циклов или изнутри вызываемых функций.

Обсуждение

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

Давайте предположим, что.

Если вы можете закодировать его с помощью try / catch, сделайте это : Чтобы прервать сложный фрагмент кода, выбрасывание исключения - правильное решение (факт, что вы можете добавить информацию о неудаче / успехе внутри вашего объект исключения не следует недооценивать). После этого у вас будет более понятный код.

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

Никто не может отрицать, что ваше решение - прославленный гото. Там не будет кода goto-spaghetti, потому что do / while не позволит вам сделать это, но это все же семантическое goto. Это может быть причиной того, что некоторые могут посчитать этот код «плохим»: они чувствуют запах goto, не находя его ключевое слово четко.

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

Заключение

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

Итак, в порядке предпочтения:

  1. Используйте try / catch
  2. Используйте goto
  3. Используйте цикл do / while
  4. Использовать ifs / nested ifs
31 голосов
/ 28 октября 2008

Вы в значительной степени просто замаскируете "goto" как фальшивую петлю. Неважно, любите ли вы gotos или нет, вы будете так же далеко впереди, используя настоящее неприкрытое goto.

Лично я бы просто написал это как

bool isGood = true;

   .... some code

   if(isGood)
   {
   .... some more code
   }

   if(isGood)
   {
   .... some more code
   }
24 голосов
/ 28 октября 2008

Зачем использовать поддельную петлю? Вы можете сделать то же самое с методом, и это, вероятно, не будет считаться «плохой практикой», поскольку это более ожидаемо.

someMethod()
{
   .... some code

   if(!isGood)
       return;

   .... some more code

   if(!isGood)
       return;

   .... some more code

 }
12 голосов
/ 28 октября 2008

У вас сложный нелинейный поток управления внутри трудно распознаваемой идиомы. Так что да, я думаю, что эта техника плохая.

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

11 голосов
/ 28 октября 2008

Это запутанно и запутанно, я бы сразу отменил это.

Рассмотрим эту альтернативу:

private void DoSomething()
{
   // some code
   if (some condition)
   {
      return;
   }
   // some more code
   if (some other condition)
   {
      return;
   }
   // yet more code
}

Также рассмотрите возможность разбиения кода выше на более чем один метод.

8 голосов
/ 28 октября 2008
public bool Method1(){ ... }
public bool Method2(){ ... }

public void DoStuff(){
    bool everythingWorked = Method1() && Method2();
    ... stuff you want executed always ...
}

Причина, по которой это работает, связана с так называемой логикой короткого замыкания. Method2 не будет вызываться, если Method1 возвращает false.

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

5 голосов
/ 28 октября 2008

То, что вы пытаетесь сделать, это нелокальное восстановление после сбоя. Вот для чего goto. Используй это. (на самом деле, это то, для чего предназначена обработка исключений - но если вы не можете использовать это, лучше всего использовать goto или setjmp / longjmp).

Этот шаблон, шаблон if(succeeded(..)) и 'goto cleanup', все 3 семантически и структурно эквивалентны. Используйте тот, который наиболее распространен в вашем проекте кода. В последовательности есть много ценности.

Я бы предостерег от if(failed(..)) break; в одном пункте: если вы попытаетесь вложить циклы, вы получите удивительный результат:

do{
   bool isGood = true;
   .... some code
   if(!isGood)
       break;
   .... some more code
   for(....){
      if(!isGood)
          break; // <-- OOPS, this will exit the 'for' loop, which 
                 //  probably isn't what the author intended
      .... some more code
   }
} while(false);
..... some other code, which has to be executed.

Ни goto cleanup, ни if(succeeded(..)) не имеют этого сюрприза, поэтому я бы рекомендовал вместо этого использовать один из этих двух.

5 голосов
/ 28 октября 2008

В основном вы только что описали goto. Я использую Goto в Си все время. Я не считаю это плохим, если только вы не используете его для эмуляции цикла (никогда не делайте этого!). Я обычно использую goto в C для эмуляции исключений (C не имеет исключений):

// Code

if (bad_thing_happened) goto catch;

// More code

if (bad_thing_happened) goto catch;

// Even more code

finally:

// This code is executed in any case
// whether we have an exception or not,
// just like finally statement in other
// languages

return whatever;

catch:

// Code to handle bad error condition

// Make sure code tagged for finally
// is executed in any case
goto finally;

За исключением того факта, что catch и, наконец, имеют противоположный порядок, я не понимаю, почему этот код должен быть ПЛОХИМ только потому, что он использует goto, если настоящий код try / catch / finally работает точно , как это и просто не использует goto. Это бессмысленно. И поэтому я не понимаю, почему ваш код должен быть помечен как BAD.

2 голосов
/ 28 октября 2008

К лучшему или худшему, я использовал конструкцию в нескольких местах. Начало этого ясно документировано, хотя:

    /* This is a one-cycle loop that simplifies error handling */
    do
    {
        ...modestly complex code, including a nested loop...
    } while (0);

Это на C, а не на C ++, поэтому исключения не возможны. Если бы я использовал C ++, я бы серьезно подумал об использовании исключений для обработки исключений. Предложенная Джереми идиома повторных испытаний также является разумной; Я использовал это чаще. RAII тоже мне очень поможет; к сожалению, C не поддерживает это легко. И использование большего количества функций поможет. Обработка разрывов внутри вложенного цикла выполняется повторным тестом.

Я бы не классифицировал это как отличный стиль; Я бы не стал автоматически классифицировать его как «ПЛОХОЙ».

2 голосов
/ 11 ноября 2008

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

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

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

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...