Отправлено: 28.07.20 19:30. Заголовок: О стиле программирования. Какой код является плохим.

Поводом для написания этой темы послужил вопрос на Stackoverflow о копировании уникальных строк из одного массива в другой и представленный код на языке программирования C в ответах на вопрос.

Для простоты я более-менее упростил все объявления и их наименования, представленные в исходном коде вопроса и в ответах за редким исключением.

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

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

 
 #include <stdio.h> 
 #include <string.h> 
  
 int main(void)  
 { 
 	enum { N = 10 }; 
 	char src[N][N] =  
 	{ 
 		"one", "three", "two", "nine", "seven", "one", "nine", "five", "three", "eight"	 
 	}; 
 	char dsn[N][N]; 
 	 
 	int indexedCount = 0;//number of unique strings 
 	int duplicates = 0; 
 	 
 	for(int i=0; i<N; i++){ 
      
 		for(int j=0; j<indexedCount; j++){ 
 			if(strcmp(src[ i ],dsn[ j ]) == 0){ 
 				duplicates = 1; 
 			} 
     		}  
      
 		if(!duplicates){ 
 			strcpy(dsn[ indexedCount ],src[ i ]); 
 			indexedCount++; 
 		} 
 		duplicates = 0; 
 	}	 
 	 
 	return 0; 
 }

Чем плох этот код?

Например, переменная duplicates нужна для использования внутри цикла for, однако, она объявлена вне цикла.

Каждая переменная должна объявляться в наименьшей области видимости, где она используется. К тому же эту переменную следует устанавливать в значение 0 в начале цикла, а не в конце его. Иначе, кто будет просматривать эти циклы, вынужден будет просматривать код в начале программы перед циклами, чтобы убедиться, что для первой итерации цикла не было забыто установить ее в 0. Кроме того для последней итерации цикла присвоение 0 этой переменной является избыточным кодом.

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

Вообще-то, эта переменная избыточная.

Есть изречение: лаконичность (или краткость) - сестра таланта. В отношении программирования его можно перефразировать как "лаконичность - сестра качественного кода". Избыточный код - это источник потенциальных багов в программе и делает чтение программы более затратным по времени.

В другом ответе был предложен следующий вариант циклов.

 
 #include <stdio.h> 
 #include <string.h> 
  
 int main(void)  
 { 
 	enum { N = 10 }; 
 	char src[N][N] =  
 	{ 
 		"one", "three", "two", "nine", "seven", "one", "nine", "five", "three", "eight"	 
 	}; 
 	char dsn[N][N]; 
  
  
 	int nUnique = 0; 
 	for (int i = 0; i < N; i++) { 
         int dup = 0;       // expect edges.startCity not to be a duplicate 
         for (int j = 0; j < nUnique; j++) { 
             if(strcmp(src[ i ],dsn[ j ]) == 0) { 
                 dup = 1;   // got a duplicate 
                 break;     // no need to go further ... 
             } 
         } 
         if (dup == 0) {    // not a duplicate: add it to cityNames 
             strcpy(dsn[ nUnique ], src[ i ]); 
             nUnique += 1; // we now have one more string 
         } 
     } 
  
 	return 0; 
 }

Как видно, здесь уже исправлен недостаток с объявлением переменной вне области ее использование. Переменная dup объявлена внутри внешнего цикла for. Однако тело внутреннего цикла for усложнено предложением break. Вполне можно было бы переписать этот цикл без использования предложения break. И, как я уже отметил, введение переменной dup является излишним.

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

Так как можно закодировать данную задачу? Ниже представлено решение, которые является наиболее лаконичным, в котором нет объявлений переменных, без которых вполне можно обойтись, потому что они дублируют свойства других переменных, которые можно использовать в заданном контексте.

 
 #include <stdio.h> 
 #include <string.h> 
  
 int main(void)  
 { 
 	enum { N = 10 }; 
 	char src[N][N] =  
 	{ 
 		"one", "three", "two", "nine", "seven", "one", "nine", "five", "three", "eight"	 
 	}; 
 	char dsn[N][N]; 
  
  
 	int nUnique = 0; 
  
 	for ( int i = 0; i < N; i++ )  
 	{ 
 		int j = 0; 
  
 		while ( j < nUnique && strcmp( src[ i ], dsn[ j ] ) != 0 )  
 		{ 
 			++j; 
 		} 
          
 		if ( j == nUnique ) strcpy( dsn[ nUnique++ ], src[ i ] ); 
 	} 
 	 
 	return 0; 
 }

В этой программе не создаются никакие дополнительные переменные для определения, имеется ли дублирующая строка или нет. Для этих целей вполне достаточно использовать переменную для внутреннего цикла j. Если массив назначения не содержит дублирующей записи, то по окончанию цикла значение этой переменной будет равно текущему числа строк в массиве назначения. Благодаря замены цикла for на цикл while мы избавились от предложения break.

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

Пишите лаконичные программы!

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

цитата:

Кроме того основной навык программиста не писать код, а читать код.

Это утверждение базируется на таком взгляде на современное программирование

цитата:

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

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

Спрашивается: зачем себе создавать трудности, чтобы затем их героически преодолевать?

Чтение плохого кода будет отнимать много времени, каким навыком чтения чужого кода вы не обладали бы. К тому же плохой код - это всегда источник многочисленных багов. А значит проект превращается в трудно поддерживаемый, а производительность программистов падает.

А поиск в гугле, как показали ответы на вопрос на Stackoverflow, описанный в данной теме, не всегда приводит к качественному и даже корректному кода.

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

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