Разработчик настаивает на том, что операторы не должны иметь отрицательных условий и всегда должны иметь блок else

У меня есть знакомый, более опытный разработчик, чем я. Мы говорили о практике программирования, и я был озадачен его подходом к заявлениям «если». Он настаивает на некоторых практиках относительно утверждений, которые я нахожу довольно странными.

Во-первых , за оператором if следует оператор else, вне зависимости от того, есть что добавить в него или нет. Что приводит к тому, что код выглядит так:

if(condition) 
{
    doStuff();
    return whatever;
}
else
{
}

Во-вторых , лучше проверять истинные значения, чем ложные. Это означает, что лучше проверять переменную doorClosed вместо переменной! DoorOpened

Его аргумент в том, что он проясняет, что делает код.

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

if(condition)
{
}
else
{
    doStuff();
    return whatever;
}

Мне кажется, что это действительно очень уродливо и / или что улучшение качества, если оно есть, незначительно. Но, будучи младшим, я склонен сомневаться в своем инстинкте.

Итак, мои вопросы: это хорошая / плохая / «не имеет значения» практика? Это обычная практика?

Явный elseблок

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

Я даже не упоминаю тот факт, что за ifутверждением не обязательно следует либо «за», elseлибо «ничего»: за ним может следовать один или несколько elifсимволов «s».

Наличие returnзаявления ухудшает положение. Даже если бы у вас действительно был код для выполнения внутри elseблока, это было бы более читабельно, чтобы сделать это так:

if (something)
{
    doStuff();
    return whatever;
}

doOtherThings();
return somethingElse;

Это заставляет код занимать на две строки меньше и освобождает elseблок от отступов . Оговорки об охране - все об этом.

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

if (something)
{
}
if (other)
{
}
else
{
}

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

Тест для true, а не дляfalse

Второе правило может иметь некоторый смысл, но не в его нынешней форме.

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

if (!this.IsMaster || (!this.Ready && !this.CanWrite))

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

Вышеуказанное условие может быть легко прочитано:

if (this.IsSlave || (this.Synchronizing && this.ReadOnly))

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

if(condition) 
{
    doStuff();
    return whatever;
}

doSomethingElse(); // no else needed
return somethingelse;

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

bool doorNotOpen = false; // a double negative is hard to reason
bool doorClosed = false; // this is much clearer

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

if(!condition)
{
    doStuff();
    return whatever;
}

1. Аргумент в пользу пустых elseвысказываний.

Я часто использую (и спорю) что-то похожее на эту первую конструкцию, пустое другое. Он сообщает читателям кода (как человека, так и инструментов автоматического анализа), что программист задумался над ситуацией. Пропавшие без вести elseзаявления, которые должны были присутствовать, убили людей, разбили транспортные средства и стоили миллионы долларов. Например, MISRA-C требует, по крайней мере, комментарий о том, что пропущенный финал еще является преднамеренным в if (condition_1) {do_this;} else if (condition_2) {do_that;} ... else if (condition_n) {do_something_else;}последовательности. Другие высокие стандарты надежности идут еще дальше: за некоторыми исключениями, пропущенные утверждения запрещены.

Единственным исключением является простой комментарий, похожий на /* Else not required */. Это сигнализирует о том же намерении, что и три строки пустые в остальном. Другое исключение, в котором пустое другое не нужно, - это то, где для читателей кода и для инструментов автоматического анализа очевидно, что это пустое еще лишнее. Например, if (condition) { do_stuff; return; }аналогично, пустое else не требуется в случае throw somethingили goto some_label¹ вместо return.

2. Аргумент в пользу предпочтения if (condition)более if (!condition).

Это элемент человеческого фактора. Сложная логическая логика запутывает многих людей. Даже опытный программист должен будет подумать if (!(complex || (boolean && condition))) do_that; else do_this;. Как минимум, переписать это как if (complex || (boolean && condition)) do_this; else do_that;.

3. Это не означает, что следует отдавать предпочтение пустым thenутверждениям.

Во втором разделе говорится «предпочитаю», а не «ты будешь». Это скорее правило, чем правило. Причина, по которой это руководство предпочитает позитивные ifусловия, заключается в том, что код должен быть ясным и очевидным. Пустое предложение then (например, if (condition) ; else do_something;) нарушает это. Это запутанное программирование, заставляющее даже самых опытных программистов выполнять резервное копирование и перечитывать ifусловие в его отрицательной форме. Поэтому сначала запишите его в форме с отрицанием и пропустите инструкцию else (или оставьте пустое else или комментарий на этот счет, если это необходимо).

¹ я писал , что тогда положение , которые заканчиваются return, throwили gotoне требует пустого еще. Очевидно, что условие else не нужно. Но как насчет goto? Кроме того, правила программирования, критически важные для безопасности, иногда запрещают досрочное возвращение и почти всегда запрещают генерировать исключения. Они, однако, позволяют gotoв ограниченной форме (например, goto cleanup1;). Это ограниченное использование gotoявляется предпочтительной практикой в ​​некоторых местах. Например, ядро ​​Linux напичкано такими gotoзаявлениями.

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

if (condition) {
    // No action needed because ...
} else {
    do_else_action()
}

if (condition) {
    do_if_action()
} else {
    // No action needed because ...
}

Но нет:

if (condition) {
    do_if_action()
} else {
    // I was told that an if always should have an else ...
}

При прочих равных предпочитают краткость.

То, что ты не пишешь, никто не должен читать и понимать.

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

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

Кроме того, избегайте написания else-ветви, если вы выходите непосредственно из if-ветви.

Полезным приложением явности будет добавление комментария всякий раз, когда вы проваливаете переключатели // FALLTHRU, и использование комментария или пустого блока, где вам нужен пустой оператор for(a;b;c) /**/;.

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

Что мне действительно не нравится в ваших примерах, так это совершенно ненужное вертикальное пространство, занимаемое пустым ELSE. Просто нет никаких причин делать это. Это ничего не добавляет к логике, не помогает документировать, что делает код, и вообще не повышает читабельность. На самом деле, я бы сказал, что добавленное вертикальное пространство может снизить читабельность.

Явный elseблок

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

На ifмой взгляд, заявление на самом деле охватывает две разные функции.

Если мы должны что-то сделать, сделайте это здесь.

Вещи как это, очевидно, не нуждаются в elseчасти.

    if (customer.hasCataracts()) {
        appointmentSuggestions.add(new CataractAppointment(customer));
    }
    if (customer.isDiabetic()) {
        customer.assignNurse(DiabeticNurses.pickBestFor(customer));
    }

и в некоторых случаях настаивание на добавлении elseможет ввести в заблуждение.

    if (k > n) {
        return BigInteger.ZERO;
    }
    if (k <= 0 || k == n) {
        return BigInteger.ONE;
    }

это не то же самое, что

    if (k > n) {
        return BigInteger.ZERO;
    } else {
        if (k <= 0 || k == n) {
            return BigInteger.ONE;
        }
    }

хотя это функционально то же самое. Запись первого ifс пустым elseможет привести вас ко второму результату, который излишне уродлив.

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

        // Count wins/losses.
        if (doors[firstChoice] == Prize.Car) {
            // We would have won without switching!
            winWhenNotSwitched += 1;
        } else {
            // We win if we switched to the car!
            if (doors[secondChoice] == Prize.Car) {
                // We picked right!
                winWhenSwitched += 1;
            } else {
                // Bad choice.
                lost += 1;
            }
        }

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

Тест для true, а не дляfalse

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

Хотя код как

    if(!customer.canBuyAlcohol()) {
        // ...
    }

раздражает читателя, но делает это

    if(customer.canBuyAlcohol()) {
        // Do nothing.
    } else {
        // ...
    }

по крайней мере так же плохо, если не хуже.

Я закодирован в BCPL много лет назад , и в этом языке есть IFпункт иUNLESS положение , чтобы вы могли закодировать гораздо более читаемы , как:

    unless(customer.canBuyAlcohol()) {
        // ...
    }

что значительно лучше, но все же не идеально.

Мой личный процесс

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

  if (returnVal == JFileChooser.APPROVE_OPTION) {
    handleFileChosen();
  } else {
    // TODO: Handle case where they pressed Cancel.
  }

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

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

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

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

Давным-давно в мэйнфреймах (например, PL / 1 и COBOL) было принято, что отрицательные проверки были менее эффективными. Это может объяснить 2-й момент, хотя в наши дни есть значительно более важные сбережения эффективности, которые игнорируются как микрооптимизации.

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

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

Однако вопрос об избежании негативов заслуживает большего внимания: как правило, когда вам нужно использовать логическое значение, вам нужны некоторые блоки кода для работы, когда оно установлено, и некоторые другие блоки для работы, когда оно не установлено. Таким образом, если вы применяете правило «без отрицательных значений», вы применяете либо наличие if() {} else {...}операторов (с пустым ifблоком!), Либо вы создаете второе логическое значение для каждого логического значения, которое содержит его отрицательное значение. Оба варианта плохи, так как они сбивают с толку ваших читателей.

Полезная политика такова: никогда не используйте отрицательную форму в логическом имени и выражайте отрицание как единое целое !. Утверждение, как if(!doorLocked)совершенно ясно, утверждение, как if(!doorUnlocked)мозги узлов. Выражение более позднего типа - это то, что вам нужно избегать любой ценой, а не присутствие одного !в if()условии.

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

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

void foo()
{
    if (condition) return;

    doStuff();
    ...
}

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

При рассмотрении аргумента «всегда иметь предложение else» есть один момент, которого я не видел ни в одном другом ответе: он может иметь смысл в стиле функционального программирования. Вроде, как бы, что-то вроде.

В стиле функционального программирования вы имеете дело с выражениями, а не с утверждениями. Таким образом, каждый блок кода имеет возвращаемое значение, включая if-then-elseвыражение. Это, однако, исключает пустой elseблок. Позвольте привести пример:

var even = if (n % 2 == 0) {
  return "even";
} else {
  return "odd";
}

Теперь, в языках с синтаксисом в стиле C или в стиле C (таких как Java, C # и JavaScript, и так далее) это выглядит странно. Однако это выглядит гораздо более знакомым, когда написано так:

var even = (n % 2 == 0) ? "even" : "odd";

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

... за оператором if следует оператор else, вне зависимости от того, есть что добавить в него или нет.

Я не согласен if (a) { } else { b(); }должен быть переписан как if (!a) { b(); }и if (a) { b(); } else { }должен быть переписан как if (a) { b(); }.

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

Во-вторых, лучше проверять истинные значения, чем ложные. Это означает, что лучше проверять переменную doorClosed вместо переменной! DoorOpened

У меня смешанные чувства по этому поводу. Недостатком наличия doorClosedи doorOpenedявляется то, что он потенциально удваивает количество слов / терминов, которые вам необходимо знать. Другим недостатком является то, что со временем значение doorClosedи doorOpenedможет измениться (другие разработчики придут за вами), и вы можете получить два свойства, которые больше не являются точными отрицаниями друг друга. Вместо того, чтобы избегать отрицаний, я ценю адаптацию языка кода (имена классов, имена переменных и т. Д.) К словарю бизнес-пользователей и к требованиям, которые мне даны. Я бы не хотел придумывать совершенно новый термин, чтобы избежать!если этот термин имеет значение только для разработчика, что бизнес-пользователи не поймут. Я хочу, чтобы разработчики говорили на одном языке с пользователями и людьми, пишущими требования. Теперь, если новые термины упрощают модель, то это важно, но это должно быть обработано до того, как требования будут завершены, а не после. Разработка должна решить эту проблему, прежде чем они начнут кодировать.

Я склонен сомневаться в своем инстинкте.

Хорошо продолжать задавать себе вопросы.

Это хорошая / плохая / "не имеет значения" практика?

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

Это обычная практика?

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

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

Однако это в некотором смысле расширенный комментарий, а не ответ.

Когда это заявлено

Во-вторых, лучше проверять истинные значения, чем ложные. Это означает, что лучше проверять переменную doorClosed вместо переменной! DoorOpened

Его аргумент в том, что он проясняет, что делает код.

С моей точки зрения, здесь делается предположение, что состояние двери - это бинарное состояние, хотя на самом деле это, по крайней мере, троичное состояние:

1. Дверь открыта.
2. Дверь не полностью открыта и не полностью закрыта.
3. Дверь закрыта.

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

1. Если датчик находится на открытой стороне двери: дверь открыта или не открыта
2. Если датчик находится на закрытой стороне двери: дверь либо закрыта, либо не закрыта.

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

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

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

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

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

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

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

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

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

Как упоминалось в другом месте, наличие блока else очень полезно, чтобы показать, что вы явно не хотите, чтобы в другом случае ничего не происходило. После выполнения большого количества функционального программирования (где еще требуется) я узнал, что вы всегда должны учитывать другое при написании if, хотя, как упоминает @OldCurmudgeon, действительно есть два разных варианта использования. Нужно иметь другое, не должно. К сожалению, это не то, что вы всегда можете сказать с первого взгляда, не говоря уже о линтере, поэтому догматично «всегда ставить блок еще».

Что касается «без негатива», опять же, абсолютные правила плохие. Наличие пустого if может быть странным, особенно если это тот тип, в котором не требуется else, поэтому пишите все это, а не! или == ложь - это плохо. Тем не менее, есть много случаев, когда негатив имеет смысл. Типичным примером будет кэширование значения:

static var cached = null

func getCached() {
    if !cached {
        cached = (some calculation, etc)
    }

    return cached
}

если фактическая (ментальная / английская) логика включает в себя отрицательную, то и утверждение if.