Стоит ли всегда писать код для других случаев, которые «никогда не произойдут»? - PullRequest
19 голосов
/ 19 марта 2010

Возьми какой-нибудь код вроде

if (person.IsMale()) {
    doGuyStuff();
} else {
    doGirlStuff();
}

Должно ли это быть написано, чтобы явно проверить, если person.isFemale(), а затем добавить новый else, который вызывает исключение? Возможно, вы проверяете значения в перечислении или что-то в этом роде. Вы думаете, что никто не будет добавлять новые элементы в перечисление, но кто знает? «Никогда не бывает» звучит как знаменитые последние слова.

Ответы [ 14 ]

9 голосов
/ 19 марта 2010

Я думаю, что вы ответили на свой вопрос. Если вы знаете , вы никогда не увидите дополнительных значений:

bool isSet = ...
if (isSet)
{
    return foo;
}
return bar;

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

8 голосов
/ 19 марта 2010

Если вы внимательно прочитаете некоторые книги по формальным методам, они предложат вам сделать такую ​​вещь.

  1. Определить постусловие

    isMale && doGuyStuff || isFemale && doGirlStuff.
    
  2. Выведите несколько заявлений кандидата, которые приведут к этому условию

    if isMale: doGuyStuff
    

    Это доказуемо приводит к некоторым пост-условиям

    if isFemale: doGirlStuff
    

    Это доказуемо ведет к некоторым постусловиям

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

  3. Вы получите следующее:

    if isMale: doGuyStuff
    elif isFemale: doGirlStuff
    

    Обратите внимание, что для предложения else нет правильного использования. Вы никогда - при формальном выводе - не получите предложение else. У вас всегда будут условия, которые являются положительными утверждениями: a && b || c && d разные вещи. В редких случаях это будет a && b || !a && c, но даже тогда вы обычно сталкиваетесь с явным условием !a.

Формально предложение «невозможное еще» должно быть ограничено выполнением чего-либо подобного следующему.

    if isMale: doGuyStuff
    elif isFemale: doGirlStuff
    else:
        raise HorrifyingSituationError

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

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

8 голосов
/ 19 марта 2010

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

4 голосов
/ 19 марта 2010

Если это НЕ МОЖЕТ произойти в производстве (но может произойти во время разработки), я использую assert:

Если это НЕ ДОЛЖНО произойти в производстве, но, возможно, это может произойти, я либо верну ошибку , либо сгенерируем исключение .

Кстати, вот небольшой хитрость в C ++, которую я где-то подхватил; поскольку многие макросы ASSERT отображают свое выражение в окне сообщения, если утверждение неверно, вы можете использовать следующую форму для ветвей, которые никогда не должны выполняться:

if (person.IsMale())
{
    AdmitEntranceToManCave();
}
else
{
    ASSERT(!"A female has gotten past our defenses. EVERYBODY PANIC!");
}

Строковый литерал вычисляется по адресу (не NULL), который логически ИСТИНА. Таким образом, использование логического оператора NOT делает его ЛОЖНЫМ.

3 голосов
/ 19 марта 2010

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

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

2 голосов
/ 19 марта 2010

Если вы хотите указать блок кода, который «не может произойти», используйте

assert (false);
1 голос
/ 19 марта 2010

Помните, что перечисления в C # могут вас укусить:

enum SwitchPosition 
{
    Off = 0,
    On = 1
}

void SetLightStatus(SwitchPosition pos)
{
    switch (pos)
    {
        case On: 
            TurnLightOn(); 
            break;
        case Off: 
            TurnLightOff(); 
            break;
        default:
            // Fugedaboudit -- will never happen, right?
    }
}

Неправильно! Звонок SetLightPosition(2); является законным и провалит все случаи в этом заявлении о переключении. Лучше всего бросить HorrifyingSituationError, как предлагает С. Лотт.

1 голос
/ 19 марта 2010

@ Michael Petrotta - ваш фрагмент кода неверен, так как выполняется действие DoY () независимо от значения истинности условия.

(извините, пока не можете добавлять комментарии ...)

1 голос
/ 19 марта 2010

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

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

0 голосов
/ 20 марта 2010

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

if(male)
{
  ...
}
else
{
  ...
}

// other stuff that nobody cares about...
assert(male||female);
...