Оформление вложенных блоков кодов.

Всегда интересовал вопрос, как правильно сделать оформление блоков кода, когда блоков среднее кол-во, допустим 5.
У каждого человека своё мнение и каждый любит писать по своему. Конечно всё зависит от ситуации, но всё же хотелось бы услышать ответ.

Вот пример, псевдокод который что-то делает и возвращает 1 в случае успеха и 0 в случае ошибки. В коде только 1 выделение памяти, но по факту может быть и больше + использование других ресурсов, которые необходимо освобождать

1) Всё вложено, результат заносится в переменную, первоначально инициализирован
Плюсы:
+ у функции одна точка входа и одна точка выхода (что является вроде как алгоритмически правильным)
+ никогда не забываем освободить память.

Минусы:
+ большая вложенность кода
+ требует первоначальную инициализацию переменной
+ использование дополнительной переменной для результата

Код (Text):

1. int MainFunct(int Param)
2. {
3.     char*   Mem;
4.     int x;
5.     int     Status = 1;
6.    
7.     if (Param != MAGIC_VAL)
8.     {
9.         Mem = malloc(MEM_SIZE);
10.         if (Mem)
11.         {
12.             for (x = 0; x < MEM_SIZE; x++)
13.             {
14.                 if (Funct(Mem, x, Param))
15.                 {
16.                     Status = 0;
17.                     break;
18.                 }
19.             }
20.            
21.             Proc1(Mem);
22.             free(Mem);
23.         }
24.     }
25.    
26.     return Status;
27. }

2) Всё вложено, результат заносится в переменную, первоначально НЕ инициализирован
Плюсы:
+ у функции одна точка входа и одна точка выхода (что является вроде как алгоритмически правильным)
+ никогда не забываем освободить память.
+ не требует инициализировать переменную
Минусы:
+ большая вложенность кода
+ использование дополнительной переменной для результата
+ увеличение объема кода

Код (Text):

1. int MainFunct(int Param)
2. {
3.     char*   Mem;
4.     int x;
5.     int     Status;
6.    
7.     if (Param != MAGIC_VAL)
8.     {
9.         Mem = malloc(MEM_SIZE);
10.         if (Mem)
11.         {
12.             Status = 1;
13.            
14.             for (x = 0; x < MEM_SIZE; x++)
15.             {
16.                 if (Funct(Mem, x, Param))
17.                 {
18.                     Status = 0;
19.                     break;
20.                 }
21.             }
22.            
23.             Proc1(Mem);
24.             free(Mem);
25.         }
26.         else
27.         {
28.             Status = 0;
29.         }
30.     }
31.     else
32.     {
33.         Status = 0;
34.     }
35.    
36.     return Status;
37. }

3) Всё вложено, результат по ходу действия
Плюсы:
+ не требует дополнительная переменная
Минусы:
+ у функции одна точка входа и множество точек выхода что затрудняет отладку
+ большая вложенность кода
+ можно забыть освободить память

Код (Text):

1. int MainFunct(int Param)
2. {
3.     char*   Mem;
4.     int x;
5.    
6.     if (Param != MAGIC_VAL)
7.     {
8.         Mem = malloc(MEM_SIZE);
9.         if (Mem)
10.         {
11.             for (x = 0; x < MEM_SIZE; x++)
12.             {
13.                 if (Funct(Mem, x, Param))
14.                 {
15.                     free(Mem);
16.                     return 0;
17.                 }
18.             }
19.            
20.             Proc1(Mem);
21.             free(Mem);
22.            
23.             return 1;
24.         }
25.     }
26.    
27.     return 0;
28. }

4) Всё разделено на отдельные блоки, результат по ходу дела
Плюсы:
+ нет большой вложенности
+ не требует дополнительная переменная
+ более понятный код
Минусы:
+ у функции одна точка входа и множество точек выхода что затрудняет отладку
+ можно забыть освободить память

Код (Text):

1. int MainFunct(int Param)
2. {
3.     char*   Mem;
4.     int x;
5.    
6.     if (Param == MAGIC_VAL)
7.     {
8.         return 0;
9.     }
10.    
11.     Mem = malloc(MEM_SIZE);
12.     if (!Mem)
13.     {
14.         return 0;
15.     }
16.    
17.     for (x = 0; x < MEM_SIZE; x++)
18.     {
19.         if (Funct(Mem, x, Param))
20.         {
21.             free(Mem);
22.             return 0;
23.         }
24.     }
25.            
26.     Proc1(Mem);
27.     free(Mem);
28.        
29.     return 1;
30. }

5) Всё разделено на отдельные блоки, для выхода используется метка
Плюсы:
+ нет большой вложенности
+ не требует дополнительная переменная
+ более понятный код
+ у функции одна точка входа и только 2 точки выхода расположенные рядом
Минусы:
+ можно забыть освободить память
+ использование меток (хотя это не минус, но всё же многие не любят), хотя Microsoft довольно часто использует метки

Код (Text):

1. int MainFunct(int Param)
2. {
3.     char*   Mem;
4.     int x;
5.    
6.     if (Param == MAGIC_VAL)
7.     {
8.         goto _Error;
9.     }
10.    
11.     Mem = malloc(MEM_SIZE);
12.     if (!Mem)
13.     {
14.         goto _Error;
15.     }
16.    
17.     for (x = 0; x < MEM_SIZE; x++)
18.     {
19.         if (Funct(Mem, x, Param))
20.         {
21.             free(Mem);
22.             goto _Error;
23.         }
24.     }
25.            
26.     Proc1(Mem);
27.     free(Mem);
28.        
29.     return 1;
30.    
31. _Error:
32.     return 0;
33. }

6) Всё разделено на отдельные блоки, для выхода используется метка и переменная с результатом
Плюсы:
+ нет большой вложенности
+ более понятный код
+ у функции одна точка входа и и одна точка выхода
Минусы:
+ можно забыть освободить память
+ использование меток
+ требуется дополнительная переменная

Код (Text):

1. int MainFunct(int Param)
2. {
3.     char*   Mem;
4.     int x;
5.     int Status = 0;
6.    
7.     if (Param == MAGIC_VAL)
8.     {
9.         goto _Exit;
10.     }
11.    
12.     Mem = malloc(MEM_SIZE);
13.     if (!Mem)
14.     {
15.         goto _Exit;
16.     }
17.    
18.     for (x = 0; x < MEM_SIZE; x++)
19.     {
20.         if (Funct(Mem, x, Param))
21.         {
22.             free(Mem);
23.             goto _Exit;
24.         }
25.     }
26.            
27.     Proc1(Mem);
28.     free(Mem);
29.     Status = 1;
30.    
31. _Exit:
32.     return Status;
33. }

Ну и так далее и тому подобные варианты.
Вот и хочется услышать что лучше использовать?

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

 

slesh

Правильно это когда среднее кол-во блоков 1.

 

Тогда маразм получается. Каждую мелочь в отдельную функцию/процедуру помещать и потом тоннами передавать параметры. Может ты не так выразился?
Я имею в виду 5 блоков - это 5 конструкций вида

Код (Text):

1. XXX
2. {
3.  
4. }

а не их глубина вложенности одного в другова

 

Ой, я вот был любителем 1/2-го вариантов, сейчас у меня смесь 1/2 + 4-го, в зависимости от ситуации, но 4-й преобладает, и вообще стараюсь избегать вложений больше 2-3х, делю на функции. Проще код читать так... Или старею...

 

slesh

А, ну ладно.

 

У меня такие приоритеты (от -10 до 10):

понятность кода 10
отсутствие утечек памяти 10
быстродействие 8
количество кода 1
малая вложенность 1
уменьшение количества переменных 0
одна точка выхода 0
использование меток -1

-10, ну например, использование глючных и тормозных сторонних библиотек

 

branvi

+1

+1

 

1. goto не использовать.
2. если блок содержит одну инструкцию, избавиться от {}
3. Использовать по возможности return value.
4. { и } всегда "одиноки" на строчке для блоков, исключение: } while (...)
5. Несколько последовательных присваиваний выравниваются так, чтобы операторы присваивания были строго друг под другом, а их позиция была кратна величине пробела по tab.

У меня вот такой какой-то стиль.

 

Меня интересует именно написание кода, а не стиль оформления блоков. Каждый человек привык к своему стилю оформления, но тут чуть другая вещь идет.

 

2slesh

сильная вложенность сильно ппц. Множественные точки выхода не проблема в принципе, особенно если используется cpp + RAII и ресурсы сами будут освобождаться при выходе за область видимости. Если мы говорим о plain C, то это проблема решается довольно просто и без goto, что-то типа

Код (Text):

1. do
2. {
3.   if(!a)
4.      break;
5.  
6.   if(!b)
7.     break;
8. }
9. while(FALSE)
10.  
11. // здесь освобождение ресурсов

do-while не рассматривается как отдельный блок.

 

2 izl3sa подобное как раз и делаю когда очень много условий и нет возможности всё раскидать по функциям

 

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

 

У меня обычно

 

В C у меня иногда такой паттерн используется:

Код (Text):

1. NTSTATUS HighlyNestedFun()
2. {
3.     NTSTATUS ntStatus = STATUS_SUCCESS;
4.     enum failure_stage {success, res1_failure, res2_failure, res3_failure, res4_failure, res5_failure}
5.         failureStage = success;
6.     HANDLE hRes1, hRes2, hRes3, hRes4, hRes5;
7.    
8.     hRes1 = AcquireResource1();
9.     if (!hRes1)
10.         ntStatus = STATUS_INSUFFICIENT_RESOURCES1, failureStage = res1_failure;
11.     else
12.     {
13.         hRes2 = AcquireResource2();
14.         if (!hRes2)
15.             ntStatus = STATUS_INSUFFICIENT_RESOURCES2, failureStage = res2_failure;
16.         else
17.         {
18.             hRes3 = AcquireResource3();
19.             if (!hRes3)
20.                 ntStatus = STATUS_INSUFFICIENT_RESOURCES3, failureStage = res3_failure;
21.             else
22.             {
23.                 hRes4 = AcquireResource4();
24.                 if (!hRes4)
25.                     ntStatus = STATUS_INSUFFICIENT_RESOURCES4, failureStage = res4_failure;
26.                 else
27.                 {
28.                     hRes5 = AcquireResource5();
29.                     if (!hRes5)
30.                         ntStatus = STATUS_INSUFFICIENT_RESOURCES5, failureStage = res5_failure;
31.                     else
32.                     {
33.                         //using acquired resources
34.                     }
35.                 }
36.             }
37.         }
38.     }
39.    
40.     switch (failureStage)
41.     {
42.     case success:
43.         FreeResource5(hRes5);
44.     case res5_failure:
45.         FreeResource4(hRes4);
46.     case res4_failure:
47.         FreeResource3(hRes3);
48.     case res3_failure:
49.         FreeResource2(hRes2);
50.     case res2_failure:
51.         FreeResource1(hRes1);
52.     case res1_failure:
53.     }
54.    
55.     return ntStatus;
56. }

Преимущества:
- унифицированное освобождение ресурсов (всё в одном месте, трудно забыть освободить что-то)
- единственная точка выхода (повышает наглядность. Множественные goto к точке выхода точно также неудобны, как и множественные точки выхода)
- вложенность не мешает, а наоборот наглядно отражает успешность выполнения функции

 

2l_inc
Куча минусов у этого подхода, кроме того, что это имхо некрасиво, так ещё и куча дополнительной работы. Вложенность мешает при добавлении нового кода другим человеком.

 

Если все дело на C++, то наверное самым красивым решением будет делать классы-обертки для выделяемых ресурсов. Обертки должны освобождать ресурс в деструкторе, если он не были освобожден принудительно. Ну а сами обертки, естественно, создавать как автоматические переменные.

 

Больше хочется знать именно в Си (так как основная работа только с ним)

 

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

 

slesh
Погугли насчёт CodingStyle ядра linux. Там чётко написано, что уровней вложенности допускается три штуки. То есть, например, функция, в ней цикл, в цикле условие. Если программисту надо большее количество уровней вложенности, значит он не имеет достаточной квалификации для написания ядерного кода. Кстати с аргументами та же фишка: если аргументов у функции пять и больше, значит функция написана кривыми руками. Ещё там написано, что в функции должен быть ровно один return.
Конечно, не стоит считать linux истиной в последней инстанции, но тот факт, что linux при таких ограничениях на код существует, всё же о чём-то говорит.

izl3sa

Нет, не решается. Если мы в функции выполняем штук пять операций, которые могут обломаться, и соответственно требуют обработки ошибок, сводящихся к освобождению ресурсов и возврату из функции, то предложенный вами do{}while(0); нисколько не выход, он лишь затрудняет чтение кода, и порождает запутанный эпилог функции, в которой будет стоять куча if'ов, в простейшей ситуации они будут выглядеть примерно так: if(file) fclose(file);. В более же сложных ситуациях придётся писать и вложенные условия и кучу другой гадости.
goto finish1; goto finish2; и тп. гораздо естественнее и понятнее. Кстати заменяет комментарии поясняющие, что данными строками преследуется единственная цель -- обработка ошибочных ситуаций. Избавляться от goto в такой ситуации можно лишь если начальство боготворит Дейкстру и считает все его высказывания непререкаемой истиной, и, соответственно, использование готу считает ересью, за которую сжигают на костре. Если же начальство более вдумчиво, то готу лучше для всех.

 

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

Код (Text):

1. int fun(int)
2. {
3.         int result = -1;
4.         char *buf1;
5.         char *buf2;
6.         char *buf3;
7.    
8.         buf1 = malloc(SIZE);
9.         if (NULL == buf1)
10.                 goto end1;
11.        
12.         buf2 = malloc(SIZE);
13.         if (NULL == buf2)
14.                 goto end2;
15.        
16.         buf3 = malloc(SIZE);
17.         if (NULL == buf3)
18.                 goto end3;
19.         . . .
20.         do_something
21.         . . .
22.        
23.         result = 0;
24.        
25.         free(buf3);
26. end3:        
27.         free(buf2);
28. end2:        
29.         free(buf1);
30. end1:  
31.         return result;
32. }