Варианты хорошего стиля?

Question

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

Хорошо, скажем, у моей программы есть 3 параметра командной строки: -a, -b и -c. Раньше я бы использовал 3 int как логические значения, скажем, aFlag, bFlag и cFlag. Затем, когда я вызываю свою processOpts( int *aFlag, int *bFlag, int *cFlag) функцию, я передаю &aFlag, &bFlag и &cFlag в качестве аргументов и устанавливаю их как *aFlag = 1.

Вот мой новый подход: иметь 1 int *options для представления всех опций и рассматривать его как массив логических значений. Итак, чтобы установить их в функции:

case 'a':
  *options |= 1;
  break;
case 'b':
  *options |= 2;
  break;
case 'c':
  *options |= 4;
  break;

Затем, вернувшись в main (или где-либо еще), когда я хочу проверить, выбрана ли опция:

if ( *options & 1 )
  // Option 'a' selected

if ( *options & 2 )
  // Option 'b' selected

if ( *options & 4 )
  // Option 'c' selected

Мой вопрос: какой метод считается лучшим стилем? Первый способ может быть более понятным и менее подверженным ошибкам, тогда как второй, вероятно, будет способствовать более легкому рефакторингу (не нужно менять прототип функции, поскольку это всего лишь один int).

Или есть еще лучший способ сделать это? :D

РЕДАКТИРОВАТЬ: добавлены перерывы в соответствии с предложением Мат.

Спасибо за все ответы, я очень впечатлен этим сообществом и его готовностью помочь всем учиться - вы, ребята, молодцы!

Answer 1

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

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

например,

enum option
{
    OPTION_none = 0,
    OPTION_a = 0x1,
    OPTION_b = 0x2,
    OPTION_c = 0x4,
};

enum option processOpts(int argc, char **argv)
{
    enum option options = OPTION_none;
    if (argc > 1)
    {
        int i;
        for (i = 1; i < argc; i++)
        {
            if (argv[i][0] == '-')
            {
                switch (argv[i][1])
                {
                case 'a': options |= OPTION_a; break;
                case 'b': options |= OPTION_b; break;
                case 'c': options |= OPTION_c; break;
                }
            }
        }
    }
    return options;
}

int main(int argc, char *argv[])
{
    enum option options = processOpts(argc, argv);
    if (options & OPTION_a)
    {
        /* do stuff for option a */
    }
    if (options & OPTION_b)
    {
        /* do stuff for option b */
    }
    if (options & OPTION_c)
    {
        /* do stuff for option c */
    }
    return 0;
}

Answer 2

Как я предложил в своем комментарии, я бы предпочел использовать битовые поля для этого.Для меня это немного более интуитивно понятное использование, позволяя компилятору выполнять большую часть работы за меня (проверка вывода сборки по крайней мере с помощью моего компилятора подтверждает, что он выполняет ожидаемые и / или операции при проверке / установке флагов).Поэтому, принимая решение Джеффа М в качестве отправной точки, я бы изменил его так:

// Assume TRUE defined as 1

struct option
{
    unsigned int OptionA : 1;  // defines 1 bit for option A flag
    unsigned int OptionB : 1;  // next bit is for option B
    unsigned int OptionC : 1;  // note, you can define more than one bit per flag
                               // if more complex options are required.
};

struct option processOpts(int argc, char **argv)
{
    struct option options = {0,0,0};  // explicit setting of all flags to false
                                      // compiler optimizes to options=0
    if (argc > 1)
    {
        int i;
        for (i = 1; i < argc; i++)
        {
            if (argv[i][0] == '-')
            {
                switch (argv[i][1])
                {
                case 'a': options.OptionA = TRUE; break;
                case 'b': options.OptionB = TRUE; break;
                case 'c': options.OptionC = TRUE; break;
                }
            }
        }
    }
    return options;
}

int main(int argc, char *argv[])
{
    struct option options = processOpts(argc, argv);
    if (options.OptionA)
    {
        /* do stuff for option a */
    }
    if (options.OptionB)
    {
        /* do stuff for option b */
    }
    if (options.OptionC)
    {
        /* do stuff for option c */
    }
    return 0;
}

Answer 3

Ваш новый подход (с переменной без знака) является обычным (и идиоматическим).

Обычно для определения 1, 2 и 4 и т. Д. Используется перечисление (илиопределить) и дать им имена

enum {
    OPTION_LEFT_TO_RIGHT = 1,
    OPTION_TOP_TO_BOTTOM = 2,
    OPTION_BOTTOM_TO_TOP = 4,
    OPTION_RIGHT_TO_LEFT = 8,
};

.

case 'a': *options |= OPTION_BOTTOM_TO_TOP; break;

.

if (*options & OPTION_RIGHT_TO_LEFT) { /* ... */ }

Answer 4

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

Однако я настоятельно рекомендую вам не использовать простые магические числа - вместо этого, сделайте что-то вроде этого:

#define OPTION_A 1
#define OPTION_B 2 ...

if (*options & OPTION_A) {
}

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

Answer 5

Это нормально - вы просто упаковываете биты в целое число - единственное, что я хотел бы сделать, это то, что 1, 2, 4 и т. Д. Должны быть символическими константами, а не буквальными жестко закодированными значениями, как указано выше, например,

enum {
    OPTION_A = 1 << 0,
    OPTION_B = 1 << 1,
    OPTION_C = 1 << 2,
};

Answer 6

Лучший способ сделать это - использовать целочисленный тип unsigned для представления набора флагов. В противном случае ваш новый подход во многих отношениях намного превосходит набор отдельных флагов.

Конечно, битовые маски, соответствующие каждой опции, должны быть представлены именованными константами вместо «магических чисел» (1, 2, 4 ...) в середине кода.

Answer 7

Обычный подход к анализу параметров командной строки заключается в использовании getopt () . Хороший пример его использования можно найти в 5 источнике .

Обратите внимание, что хотя getopt () соответствует POSIX.2 и POSIX.1-2001, getopt_long () является расширением GNU, и следует избегать IMO.

Answer 8

То, как вы извились, - это выдающийся путь. Это может быть даже «лучший» способ. Поздравляем! Молодец.

- Пит