Используйте else после исключения (или нет)

9

Рассмотрим этот фрагмент кода:

if (x == 1)
{
  throw "no good; aborting" ;
}

[... more code ...]

Теперь рассмотрим этот код:

if (x == 1)
{
  throw "no good; aborting" ;
}
else
{
  [... more code ...]
}

Два случая работают точно так же. Преимущество первого случая состоит в том, что вам не нужно «заключать» остальную часть кода в else. Второе преимущество заключается в следовании практике явного наличия elseкаждого if.

Может ли кто-нибудь привести веские аргументы в пользу одного над другим?

rlandster
источник
5
Практика, которую вы цитируете в пользу явного, elseкажется фальшивой. Довольно часто просто нечего положить в elseблок, если вы не наклонитесь назад.
Последовательность? Разрешить надежность перед лицом изменения кода? Читаемость?
Томас Эдинг
1
Эхххх, я не такой большой поклонник идеи, которая ifнужна каждому else. Последний программист, который работал над нашей кодовой базой, строго следовал этому ( ну, иногда ... это отчасти шизофренично ). В результате у нас есть много совершенно бессмысленных else { /* do nothing */ }блоков, засоряющих код ...
KChaloux
4
«Другое для каждого, если» выглядит как какое-то странное заявление, выпущенное облачным архитектором во имя (глупой) последовательности. Я не вижу никакого преимущества в следовании этой практике и никогда о ней даже не слышал.
Эрик Дитрих
Это избыточно еще. Если вы работаете со стеком .NET, установите ReSharper, и он напомнит вам удалить все лишние операторы else.
CodeART

Ответы:

16

Не следует добавлять elseпосле ifветок, которые безоговорочно нарушают поток управления , таких как ветки , содержащие a throwили a return. Это улучшит читабельность вашей программы, удалив ненужный уровень вложенности, введенный elseветкой.

То, что выглядит более-менее нормально с одним, throwстановится по-настоящему уродливым, когда включается несколько бросков подряд:

void myMethod(int arg1, int arg2, int arg3) {
    // This is demonstrably ugly - do not code like that!
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    } else {
        if (!isValid(arg2)) {
            throw new ArgumentException("arg2 is invalid");
        } else {
            if (!isValid(arg3)) {
                throw new ArgumentException("arg3 is invalid");
            } else {
                // The useful code starts here
            }
        }
    }
}

Этот фрагмент делает то же самое, но выглядит намного лучше:

void myMethod(int arg1, int arg2, int arg3) {
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    }
    if (!isValid(arg2)) {
        throw new ArgumentException("arg2 is invalid");
    }
    if (!isValid(arg3)) {
        throw new ArgumentException("arg3 is invalid");
    }
    // The useful code starts here
}
dasblinkenlight
источник
+1 верно. Второй случай OP заставляет вас внимательно читать, а затем оставляет вас с WTF. Но ... всегда старайтесь сделать методы короткими. Возвращение в середине метода 200 строк тоже плохо.
Тулаинс Кордова
1
Чтобы быть справедливым, если вы просто используете повторный, если вы можете сделать else if.
Гуванте
2
@Guvante: Каждый ifтестирует одно условие и имеет дело с ним, если условие истинно, и если не существует какой-то альтернативы, которая должна произойти, если условие ложно, else ifs не нужны. У нас есть термин вокруг моего офиса для кода , как первый фрагмент dasblinkenlight в: « пачинко машины.»
Blrfl
@Blrfl Pachinko машины ха, идеальная аналогия +1
Джимми Хоффа
@Blrfl: я ссылался на то, что повторяющиеся if являются плохим примером слишком большого количества вложений. Вы не должны вложить повторяющиеся если так или иначе. Я согласен, что в целом, если вы не говорите о небольшом количестве кода, нет причин для включения else.
Гуванте
5

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

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

Скажем, например, у вас есть эта функция:

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
    }
    else
    {
        oblogonToConfigure.Color = _validColors[0];
        oblogonToConfigure.ColorIndex = 0;
    }
}

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

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validOblogons.Contains(oblogonToConfigure.Type))
    {
        oblogonToConfigure.Type = _validOblogons[0];
        oblogonToConfigure.TypeIndex = 0;
        if (_validColors.Contains(oblogonToConfigure.Color))
        {
            oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
        }
        else
        {
            oblogonToConfigure.Color = _validColors[0];
            oblogonToConfigure.ColorIndex = 0;
        }
    }
    else
    {
        oblogonToConfigure.TypeIndex = _validOblogons.IndexOf(oblogonToConfigure.Type);
    }
}

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

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.Color = _validColors[0];
    }

    oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
}

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

Я знаю, пример немного надуманный, но я много раз видел

SomeFunction()
{
    if (isvalid)
    {
        /* ENTIRE FUNCTION */
    }
    /* Nothing should go here but something does on accident, and an invalid scenario is created. */
}

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

SomeFunction()
{
    if (!isvalid)
    {
        /* Nothing should go here, and it's so small no one will likely accidentally put something here */
        return;
    }

    /* ENTIRE FUNCTION */
}
Джимми Хоффа
источник