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

Question

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

Мой код выглядит так:

static void RequestHandler(bool isThisRequestANewRequest)
{
    typedef enum
    {
        STATE_NEW_REQUEST,
        STATE_1,
        STATE_2,
        STATE_ERROR,
    } States;

    static States  state = STATE_ERROR;

    if(isThisRequestANewRequest == true)
    {
        state = STATE_NEW_REQUEST;
    }

    switch(state)
    {    
        case STATE_NEW_REQUEST:
            //init request flags
            state = STATE_1;
            //lint -fallthrough
        case STATE_1:
            //do something
            break;
        case STATE_2:
            //do something else
            break;
        case STATE_ERROR:
        default:
            //do something in case of error
            break;
     }
}         

Это можно считать хорошей практикой? Есть ли риск? Есть ли минусы? (обслуживание, чтение, ...)

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

Примечание. Мой вопрос касается как однопоточных, так и многопоточных задач.

Answer 1

Как правило, максимально возможное уменьшение объема является хорошей практикой.Если вам нужна какая-то каноническая ссылка на это, то самое близкое, что я могу придумать, это MISRA-C: 2012, правило 8.9, которое рекомендует объявлять объекты, которые используются только одной функцией, в области видимости блока.Я не понимаю, почему то же самое не относится к типам.

Однако не стоит полагаться на провал в операторах switch, поскольку это блокирует статические анализаторы (например, Lint в данном случае) от поискареальные ошибки, вызванные отсутствием break.Это также затрудняет чтение и сопровождение кода - лично я бы посоветовал использовать сквозные переключатели гораздо хуже, чем повторение кода.

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

if(state == STATE_NEW_REQUEST)
{
  ...
  state = STATE_1;
}
if(state == STATE_1)
{
  ...
}

В противном случае, если вы выполняете только одно состояние для вызова функции, вы можете использовать switch.В общем, необходимость «выполнять несколько состояний на состояние» является намеком на то, что можно улучшить дизайн более широкой программы.