Избегайте дублирования кода - PullRequest
9 голосов
/ 25 июня 2010

скажем, у меня есть:

  switch( choice ) {
  case A:   
     stmt;
     do_stmt_related2A;
  break;

  case B:
     stmt;
     do_stmt_related2B;
  break;

  case C: something_different();
   ...
  }

Как можно избежать дублирования кода stmt?

Но есть ли обходной путь?Метка расширения gcc как значение выглядит довольно хорошо для такой ситуации.

   switch( choice ) {
     do {
     case A:  ptr = &&A_label;
     break;
     case B:  ptr = &&B_label;
     } while(0);
              stmt;
              goto *ptr;
     case C: ...

Есть ли какой-нибудь прием, который может сделать то же самое в ANSI-C?Изменить: Конечно, я подумал о функции / макрос / встроенный.Но что-нибудь еще?Дело не в производительности тоже.Просто для образовательных целей.;)

Ответы [ 15 ]

29 голосов
/ 25 июня 2010

Почему бы вам просто не сделать рефакторинг stmt (я полагаю, что это большой кусок инструкций, а не одной строки) в свою собственную функцию do_stmt() и вызвать ее?Что-то вроде:

switch( choice ) {
    case A:
        do_stmt();
        do_stmt_related2A;
        break;
    case B:
        do_stmt();
        do_stmt_related2B;
        break;
    case C: something_different();
        ...
}

Этот трюк с gcc действительно отвратителен.Я предпочел бы иметь читаемый код над такими чудовищами в любой день.

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

17 голосов
/ 25 июня 2010

Как мне избежать дублирования кода stmt?

Поместив его в функцию и вызвав ее.

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

9 голосов
/ 01 июля 2010

Помимо общего мнения о необходимости рефакторинга общего кода в подфункцию, простейший способ реорганизации вашего кода с использованием стандартных функций C, вероятно, таков:

if (choice == A || choice == B) {
    stmt;
}

switch( choice ) {
  case A:   
    do_stmt_related2A;
    break;

  case B:
    do_stmt_related2B;
    break;

  case C:
    something_different();
    ...
}

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

4 голосов
/ 25 июня 2010

В любом случае будет некоторый код - вы можете иметь дублированный код или код, чтобы избежать дублирования. Поэтому мне любопытно, насколько сложен код stmt; на самом деле.

Простое, чистое решение - переместить разделяемую часть (stmt) в отдельную функцию.

void do_shared_stmt(void) {
 stmt;
}
/* .... */
swtich(choise) {
case A:
  do_shared_stmt();
  do_stmt_related2A();
  break;
case B:
  do_shared_stmt();
  do_stmt_related2B();
  break;
case C:
  something_different();
/* ... */
}

Другое решение (которое может быть приемлемым, в зависимости от вашей ситуации) заключается во вложении операторов ветвления:

swtich(choise) {
case A:
case B:
  stmt;
  if(choise == A) {
    do_stmt_related2A();
  } else {̈́
    do_stmt_related2B();
  }
  break;
case C:
  something_different();
/* ... */
}
1 голос
/ 30 июня 2010

Я, наверное, сделаю что-то подобное:

void do_stmt(int choice)
{
    stmt;
    switch(choice)
    {
         case A:
             do_stmt_related2A;
             break;
         case B:
             do_stmt_related2B;
             break;
    }  
}
/* ... */
switch(choice)
{
    case A:
    case B:
        do_stmt(choice);
        break;
    case C:
         something_different();
...
0 голосов
/ 04 ноября 2010

Вот вероятность, которую вы, вероятно, должны игнорировать (но это довольно интересно):

// function pointer to B function
void (*func)(void) = do_stmt_related2B;
switch(choice) {
case A:
    func = do_stmt_related2A; // change function pointer to A function
case B:
    stmt;
    func(); // call function pointer
    break;

case C:
    something_different();
    ...
}

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

0 голосов
/ 06 июля 2010

Другие уже дали этот ответ, но я хотел бы сформулировать его в формальных терминах.

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

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

0 голосов
/ 06 июля 2010

Я вижу только два возможных варианта вашей логики.

L1:
 val
 |
 |-------------------
 |        |         |
 A        B         C
 |        |         |
 stmt     stmt      crap
 |        |
 Afunk    Bfunk

L2:
 val
 |
 |-------------------
 stmt               |
 |---------         |
 |        |         |
 A        B         C
 |        |         |
 Afunk    Bfunk     crap

В L1 используйте встроенную функцию для stmt или используйте L2, если ветвление дважды в порядке.Если вы говорите о дублировании кода src (в отличие от дублирования в двоичном коде), просто встроенный func решит вашу проблему.

0 голосов
/ 05 июля 2010

Вы можете просто вставить свой код stmt в функцию и вызывать эту функцию из своего дела или где угодно, вы можете вызывать функцию сколько угодно раз.

0 голосов
/ 04 июля 2010

Вернитесь к первоначальным значениям различных значений регистра. Что представляют собой случаи A, B и C? Если A и B должны выполнять частично один и тот же код, они должны иметь некоторое сходство. Найди их и вырази по-другому.

Следующий пример проясняет ситуацию. Предположим, что A, B и C - это 3 разных вида животных, и дублированный код для A и B фактически типичен для животных, которые могут летать. Вы можете написать свой код так:

if (can_fly(choice))
   {
   stmt;
   }

switch( choice )
  {
  case A:   
     do_stmt_related2A;
     break;
  case B:
     do_stmt_related2B;
     break;
  case C:
     something_different();
     break;
  }

Таким образом, если третий вариант D будет добавлен позже в ваше приложение, шансы забыть дублирующийся код "stmt" будут немного меньше.

Если вы действительно хотите предотвратить вызов функции (to can_fly), а A, B и C являются числовыми константами, вы можете использовать битовые маски. В этом примере мы используем один бит, чтобы указать, что животное может летать:

if (choice & BIT_CAN_FLY)
   {
   stmt;
   }

switch( choice )
  {
  case A:   
     do_stmt_related2A;
     break;
  case B:
     do_stmt_related2B;
     break;
  case C:
     something_different();
     break;
  }
...