Какой стиль кодирования лучше? - PullRequest
12 голосов
/ 29 октября 2009

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

Ниже приведены несколько упрощенных примеров кода.

Уплотненный:

If(condition1)
{
    If(condition2)
    {
        if(condition3)
        {
            return true;
        }
        else
        {
            log("condition3 failed");
        }
    else
    {
        log("condition2 failed")
    }
}
else
{
    log("condition1 failed")
}

return false;

или

Bool Driven:

bool bRC = false;

bRC = (condition1);
if(brc)
{
    bRC = (condition2);
}
else
{
    log("condition1 failed");
    return false;
}

if(bRC)
{
    bRC = (condition3);
}
else
{
    log("condition2 failed");
    return false;
}

if(bRC)
{
    return true;
}
else
{
    log("condition3 failed");
    return false;
}

Ответы [ 14 ]

32 голосов
/ 29 октября 2009

Мне нравится твой лучше, но я бы, наверное, сделал что-то вроде:

if (condition1 && condition2 && condition3)
{
    return true;
}
else if (!condition1)
{
    log("condition1 failed");
}
else if (!condition2)
{
    log("condition2 failed");
}
else
{
    log("condition3 failed");
}
return false;

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

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

19 голосов
/ 29 октября 2009

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

if(!condition1)
{
    log("condition1 failed");
    return false;
}

if(!condition2)
{
    log("condition2 failed");
    return false;
}

if(!condition3)
{
    log("condition3 failed");
    return false;
}

return true;

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

if(!condition1)
    log("condition1 failed");

else if(!condition2)
    log("condition2 failed");

else if(!condition3)
    log("condition3 failed");

else
    return true;

return false;

Но это довольно закулисно и менее "ясно", на мой взгляд.

18 голосов
/ 29 октября 2009

Лично мне гораздо проще читать вложенный код.

6 голосов
/ 29 октября 2009

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

5 голосов
/ 29 октября 2009

Похоже на вложенную версию, но намного чище для меня:

if not condition1:
    log("condition 1 failed")
else if not condition2:
    log("condition 2 failed")
else if not condition3:
    log("condition 3 failed")
else
    return true;
return false;

Помните, что каждое условие оценивается один раз.

3 голосов
/ 29 октября 2009

Второй стиль абсурдно многословен: он действительно предлагал именно это? Вам не нужно большинство из этих if операторов, потому что в остальной части есть return.

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

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

2 голосов
/ 29 октября 2009

Я думаю, что оба пути возможны и имеют свои плюсы и минусы. Я бы использовал стиль, управляемый bool, в случаях, когда у меня была бы действительно глубокая вложенность (8 или 10 или что-то в этом роде) В вашем случае с 3 уровнями, я бы выбрал ваш стиль, но для точного примера из вышеприведенного, я бы пошел так:

void YourMethod(...)
{
  if (condition1 && condition2 && consition3)
    return true;

  if (!condition 1)
    log("condition 1 failed");

  if (!condition 2)
    log("condition 2 failed");

  if (!condition 3)
    log("condition 3 failed");

  return result;
}

... или так, если вы предпочитаете одну точку выхода (как я) ...

void YourMethod(...)
{
  bool result = false;

  if (condition1 && condition2 && consition3)
  { 
    result = true;
  }
  else
  {
    if (!condition 1)
      log("condition 1 failed");

    if (!condition 2)
      log("condition 2 failed");

    if (!condition 3)
      log("condition 3 failed");
  }
  return result;
}

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

2 голосов
/ 29 октября 2009

Способ, которым управляет bool, сбивает с толку. Вложение хорошо, если требуется, но вы можете удалить часть глубины вложения, объединив условия в один оператор или вызвав метод, в котором выполняется некоторая дополнительная оценка.

1 голос
/ 29 октября 2009

Мне не нравится ни один из способов. Когда у вас так много гнезд, что-то не так. В случае проверки формы или чего-то, что действительно требует чего-то подобного, попытайтесь выяснить что-то более модульное или компактное.

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

В зависимости от ваших потребностей существует слишком много реализаций, поэтому создание примера кода было бы бессмысленным.

Как правило, когда ваш код выглядит слишком сложным, он отстой :). Попробуйте переосмыслить это. Следование практике кодирования в большинстве случаев делает код более эстетичным и коротким; и, очевидно, также умнее.

1 голос
/ 29 октября 2009

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

if (!condition1) {
    log("condition1 failed");
    return false;
}
if (!condition2) {
    log("condition2 failed");
    return false;
}
if (!condition3) {
    log("condition3 failed");
    return false;
}
return true;

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

result = true;
if (!condition1) {
    log("condition1 failed");
    result = false;
}
if (!condition2) {
    log("condition2 failed");
    result = false;
}
if (!condition3) {
    log("condition3 failed");
    result = false;
}
return result;
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...