Оператор переключения с возвратами - корректность кода

91

Скажем, у меня есть код на C примерно с такой структурой:

switch (something)
{
    case 0:
      return "blah";
      break;

    case 1:
    case 4:
      return "foo";
      break;

    case 2:
    case 3:
      return "bar";
      break;

    default:
      return "foobar";
      break;
}

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

Что вы думаете? Их можно удалить? Или оставите их для большей «правильности»?

Houbysoft
источник

Ответы:

139

Удалите breakутверждения. Они не нужны, и, возможно, некоторые компиляторы выдадут предупреждения «Недостижимый код» .

Кгианнакакис
источник
2
То же самое относится и к другим операторам безусловного перехода к управлению, например continueили goto- идиоматично использовать их вместо break.
caf
32

Я бы пошел совершенно другим путем. НЕ ВОЗВРАЩАЙТЕСЬ в середине метода / функции. Вместо этого просто поместите возвращаемое значение в локальную переменную и отправьте его в конце.

Лично я нахожу более читаемым следующее:

String result = "";

switch (something) {
case 0:
  result = "blah";
  break;
case 1:
  result = "foo";
  break;
}

return result;
Не я
источник
24
Философия «One Exit» имеет смысл в C, где вам нужно убедиться, что все очищено правильно. Учитывая, что в C ++ вас могут вырвать из функции в любой момент из-за исключения, философия одного выхода не очень полезна в C ++.
Билли Онил
29
Теоретически это отличная идея, но часто требуются дополнительные блоки управления, которые могут ухудшить читаемость.
mikerobi
8
Основная причина, по которой я поступаю таким образом, заключается в том, что в более длинных функциях очень легко пропустить вектор выхода при сканировании кода. Однако, когда выход всегда в конце, легче быстро понять, что происходит.
NotMe
7
Ну, это и блоки возврата в середине кода просто напоминают мне о злоупотреблении GOTO.
NotMe
5
@Chris: Что касается более длинных функций, я полностью согласен, но это обычно говорит о более серьезных проблемах, поэтому реорганизуйте его. Во многих случаях это тривиальная функция, возвращающаяся при переключении, и очень ясно, что должно произойти, поэтому не тратьте силы мозга на отслеживание дополнительной переменной.
Стивен
8

Лично я бы убрал возвраты и оставил перерывы. Я бы использовал оператор switch для присвоения значения переменной. Затем верните эту переменную после оператора switch.

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

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

Amardeep AC9MF
источник
Может быть хорошо для этого switch, но не обязательно в целом. Что ж, допустим, оператор switch внутри функции предшествует еще 500 строкам кода, который действителен только в определенных случаях true. Какой смысл выполнять весь этот дополнительный код для случаев, когда его выполнять не нужно; не лучше ли просто returnвнутри switchдля тех case?
Fr0zenFyr
6

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

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

Пол Р
источник
5

Удалить их. Это идиоматично возвращаться из caseоператоров, иначе это шум «недоступного кода».

Стивен
источник
5

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

Хэнк Гей
источник
4

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

Конечно, я бы также рассмотрел что-то вроде:

char const *rets[] = {"blah", "foo", "bar"};

return rets[something];

Изменить: даже с отредактированным сообщением эта общая идея может работать нормально:

char const *rets[] = { "blah", "foo", "bar", "bar", "foo"};

if ((unsigned)something < 5)
    return rets[something]
return "foobar";

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

Джерри Гроб
источник
Отредактировал сообщение, чтобы отразить, почему это решение не работает.
Houbysoft
@your edit: Да, но для этого потребуется больше памяти и т. д. ИМХО, переключатель - это самый простой способ, и это то, что я хочу в такой маленькой функции (он выполняет только переключатель).
Houbysoft
3
@houbysoft: внимательно посмотрите, прежде чем сделать вывод, что потребуется дополнительная память. В типичном компиляторе использование «foo» дважды (например) будет использовать два указателя на один блок данных, а не реплицировать данные. Вы также можете ожидать более короткий код. Если у вас нет много повторяющихся значений, это часто позволяет сэкономить память в целом.
Джерри Коффин
правда. Я по-прежнему буду придерживаться своего решения, потому что список значений очень длинный, и переключатель кажется лучше.
Houbysoft
1
В самом деле, более элегантный и эффективный ответ на подобную проблему, когда значения переключателей являются смежными, а тип данных однородным, - это использование массива без переключения в целом.
Дуэйн Робинсон
2

Я бы сказал, удалите их и определите ветку default:.

Белла
источник
Если разумного значения по умолчанию нет, вам не следует определять значение по умолчанию.
Билли Онил
есть один, и я его определил, просто в вопрос не ставил, отредактировал сейчас.
Houbysoft
2

Не лучше ли иметь массив с

arr[0] = "blah"
arr[1] = "foo"
arr[2] = "bar"

и делать return arr[something];?

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

Вивин Палиат
источник
Отредактировал сообщение, чтобы отразить, почему это решение не работает.
houbysoft
2

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

Стив Шелдон
источник
2

Что вы думаете? Их можно удалить? Или оставите их для большей «правильности»?

Их можно удалить. Использование return - это именно тот сценарий, в котором breakего не следует использовать.

OscarRyz
источник
1

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

Я знаю, что это не так, когда returnвместо a есть a break, но именно так мои глаза «читают» блоки case в переключателе, поэтому я лично предпочел бы, чтобы каждый из них caseбыл соединен с a break. Но многие компиляторы жалуются на то, что breakafter returnявляется излишним / недоступным, и, видимо, я все равно в меньшинстве.

Так что избавьтесь от breakследующих a return.

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

Майкл Берр
источник
0

Я говорю: уберите их. Если ваш код настолько нечитабелен, что вам нужно втыкать в него перерывы «на всякий случай», вам следует пересмотреть свой стиль кодирования :)

Кроме того, я всегда предпочитал не смешивать в операторе switch разрывы и возвраты, а придерживаться одного из них.

Томас Кьёрнес
источник
0

Я лично склонен терять breaks. Возможно, одним из источников этой привычки является программирование оконных процедур для приложений Windows:

LRESULT WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
{
    switch (uMsg)
    {
        case WM_SIZE:
            return sizeHandler (...);
        case WM_DESTROY:
            return destroyHandler (...);
        ...
    }

    return DefWindowProc(hwnd, uMsg, wParam, lParam);
}

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

Mac
источник
0

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

Бли, идея очень проста:


Switch ( Algorithm )
{

 case 1:
 {
   Call_911;
   Jump;
 }**break**;
 case 2:
 {
   Call Samantha_28;
   Forget;
 }**break**;
 case 3:
 {
   Call it_a_day;
 }**break**;

Return thinkAboutIt?1:return 0;

void Samantha_28(int oBed)
{
   LONG way_from_right;
   SHORT Forget_is_my_job;
   LONG JMP_is_for_assembly;
   LONG assembly_I_work_for_cops;

   BOOL allOfTheAbove;

   int Elligence_says_anyways_thinkAboutIt_**break**_if_we_code_like_this_we_d_be_monkeys;

}
// Sometimes Programming is supposed to convey the meaning and the essence of the task at hand. It is // there to serve a purpose and to keep it alive. While you are not looking, your program is doing  // its thing. Do you trust it?
// This is how you can...
// ----------
// **Break**; Please, take a **Break**;

/ * Это небольшой вопрос. Сколько кофе вы выпили, читая вышесказанное? ИТ иногда ломает систему * /

Cruentos Solum
источник
0

Код выхода в одной точке. Это обеспечивает лучшую читаемость кода. Добавление операторов возврата (множественных выходов) между ними затруднит отладку.

антиш
источник
0

Если у вас есть код типа «поиск», вы можете упаковать предложение switch-case в метод отдельно.

У меня есть несколько из них в системе «хобби», которую я разрабатываю для развлечения:

private int basePerCapitaIncomeRaw(int tl) {
    switch (tl) {
        case 0:     return 7500;
        case 1:     return 7800;
        case 2:     return 8100;
        case 3:     return 8400;
        case 4:     return 9600;
        case 5:     return 13000;
        case 6:     return 19000;
        case 7:     return 25000;
        case 8:     return 31000;
        case 9:     return 43000;
        case 10:    return 67000;
        case 11:    return 97000;
        default:    return 130000;
    }
}

(Ага. Это пространство GURPS ...)

Я согласен с другими, что в большинстве случаев вам следует избегать более одного возврата в методе, и я действительно понимаю, что это могло бы быть лучше реализовано как массив или что-то еще. Я только что обнаружил, что switch-case-return довольно легко сопоставить с поисковой таблицей с корреляцией 1-1 между вводом и выводом, как в вышеупомянутой вещи (ролевые игры полны их, я уверен, что они существуют в других "бизнес" тоже): D

С другой стороны, если предложение case более сложное или что-то происходит после оператора switch, я бы не рекомендовал использовать в нем return, а скорее установил бы переменную в переключателе, завершил его перерывом и вернул значение переменной в конце.

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

Эрк
источник