2017-01-13 5 views
2

Чистые принципы кодирования обычно включают правило, что функции должны быть небольшими и одноцелевыми.Чистое кодирование: как разбить петли внутри вызова функции внутри петли

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

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

for (arow = row, acol = col - 1, found_obstacle = false; acol >= 0; --acol) { 
    if (!found_obstacle) { 
    if (cellHasUnit(arow, acol)) found_obstacle = true; 
    continue; 
    } else { 
    if (cellHasUnit(arow, acol)) { 
     if (!cellHasAlly(arow, acol)) 
     _dangerzones.insert(std::make_pair(arow, acol)); 
     break; 
    } 
    } 
} 

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

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

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

+2

это вопрос для http://codereview.stackexchange.com/ –

+0

Параметр 'continue' является ненужный (и неправильно отступ).Инвертирование теста на 'cellHasUnit' упростит цикл. –

+0

В качестве альтернативы кодосмотру, рассмотрите programers.stackexchange. –

ответ

2

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

Что касается вашего кода, все в порядке. Не сложно. Вы используете функции внутри цикла и выглядят модульными. Перерыв тоже в порядке.

У меня есть один комментарий, однако, используя continue выглядит излишним. Вы можете сделать:

if (cellHasUnit(arow, acol)) { 
    found_obstacle = true; 
else { 
    ... 

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

+0

+1 от меня. Я думаю, что Мартин не должен предлагать свои руководящие принципы как «правила». Слишком много функций не хуже, чем слишком мало. –

1

Возможно, в инструкции break; установите acol = -1, а затем continue;, чтобы на следующей итерации вы вырвались из цикла?

+0

Спасибо, и вы правы, но в интересах написания чистого кода я этого не делал: я думал, что это будет запутывать мои намерения. – Anton

0

Вы можете найти эту форму более выразительно. Он переворачивает тесты и поэтому избегает 2 места вызова для теста cellHasUnit:

#include <map> 

std::map<int, int> _dangerzones; 
bool cellHasUnit(int, int); 
bool cellHasAlly(int, int); 

void handleAlly(int arow, int acol) 
{ 
    if (!cellHasAlly(arow, acol)) 
    _dangerzones.insert(std::make_pair(arow, acol)); 
} 

void test(int row, int col) 
{ 
    int arow = row; 
    bool found_obstacle = false; 
    for (int acol = col - 1 ; acol >= 0 ; --acol) 
    { 
     if (cellHasUnit(arow, acol)) 
     { 
      if (found_obstacle) 
      { 
       return handleAlly(arow, acol); 
      } 
      else // not strictly necessary 
       found_obstacle = true; 
     } 
    } 
} 

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

Если реальная функция больше, то вы могли бы написать вместо этого:

  if (found_obstacle) 
      { 
       handleAlly(arow, acol); 
       break; 
      } 
      else ...