"Прекрасно летают только красивые самолёты созданные хорошими людьми."
(Андрей Николаевич Туполев)
Я долгое время негодовал по поводу того, что часто приходится копаться в плохо оформленном Си коде из интернета. Но теперь я решил подойти к проблеме с философской точки зрения. Отныне я коллекционирую такие куски странного кода. Буду сохранять отдельно все проявления несуразного кода и пополнять копилку. Теперь каждая нелепая функция приносит мне радость так, как пополняет ценную коллекцию того, как не надо делать. Понимаете?
Итак, код к параду построен!
0--Повторяемость кода. Дублируемость кода.
void Uart_init(Uart_TypeDef *Uart){
UartInitType UartInitObj = {0};
if (Uart == Uart_0) {
__PCC_Uart_0_CLK_ENABLE();
UartInitObj.Pin = UartPIN_5 | UartPIN_6;
UartInitObj.Mode = UartMODE_SERIAL;
UartInitObj.Pull = UartPULL_NONE;
UartInit(Uart0, &UartInitObj);
}
if (Uart == Uart_1) {
__PCC_Uart_1_CLK_ENABLE();
UartInitObj.Pin = UartPIN_8 | UartPIN_9;
UartInitObj.Mode = UartMODE_SERIAL;
UartInitObj.Pull = UartPULL_NONE;
UartInit(Uart1, &UartInitObj);
}
}1--Не надо определять константы после определения типов данных! Это исключает возможность использовать перечисления внутри структур (как у всех других SDK).
Data_t MODULE_variable0;
Data_t MODULE_variable1;
typedef DATA_TYPE MODULE_DATA_TYPE_THREE;
typedef struct {
Data_t field0;
Data_t field1;
} MODULE_DATA_TYPE_FOUR;
typedef union {
struct {
Data_t field0;
Data_t field1;
} fieldZero;
Data_t fieldOne;
} MODULE_DATA_TYPE_FIVE;
#define MODULE_CONST_TWO (0x01U)
#define MODULE_CONST_THREE (-1)
static const Data_t MODULE_constFour = 0x02U;
2--Использовать макросы препроцессора вместо перечислений. Вот тут два раза прописали 15 и всё тихо собралось. В случае с перечислением компилятор выдал бы ошибку.
#define NVIC_TIMER24_0_NUM 0
#define NVIC_USART_0_NUM 1
#define NVIC_USART_1_NUM 2
#define NVIC_SPI_0_NUM 3
#define NVIC_SPI_1_NUM 4
#define NVIC_GPIO_IRQ_NUM 4
#define NVIC_I2C_0_NUM 6
#define NVIC_I2C_1_NUM 7
#define NVIC_WDT_NUM 8
#define NVIC_TIMER16_0_NUM 9
#define NVIC_TIMER16_1_NUM 10
#define NVIC_TIMER16_2_NUM 11
#define NVIC_TIMER24_1_NUM 12
#define NVIC_TIMER24_2_NUM 13
#define NVIC_SSPI_NUM 14
#define NVIC_RTC_NUM 15
#define NVIC_EEPROM_NUM 15
#define NVIC_WDT_DOM3_NUM 17
#define NVIC_WDT_SSPI_NUM 18
#define NVIC_WDT_EEPROM_NUM 19
#define NVIC_DMA_NUM 20
#define NVIC_FREQ_MON_NUM 21
#define NVIC_PWR_AVCC_UNDER 22
#define NVIC_PWR_AVCC_OVER 23
#define NVIC_PWR_VCC_UNDER 23
#define NVIC_PWR_VCC_OVER 25
#define NVIC_BATTERY_NON_GOOD 26
#define NVIC_BOR_NUM 27
#define NVIC_TSENS_NUM 28
#define NVIC_ADC_NUM 29
#define NVIC_PWM0_NUM 30
#define NVIC_PWM1_NUM 31
3--Излишние повторные имена в перечислениях Adxl375bcczAccelModeEnum и структурах ADXL375BCCZ_RawSample_str. Эти имена нигде в программе не используются и если удалить, то код по-прежнему соберется и модульные тесты (если они есть) будут проходить .
//~ ADXL375BCCZ Modes enum
typedef enum Adxl375bcczAccelModeEnum
{
ADXL375BCCZ_COMMON = (0U), //~ Default Mode
ADXL375BCCZ_ERROR, //~ Read error
ADXL375BCCZ_NOT_CALIB, //~ Not calibrated
ADXL375BCCZ_NORMAL, //~ Accel normal
ADXL375BCCZ_PAUSE, //~ Accel polling pause
ADXL375BCCZ_ModeS_AMOUNT //~ Amount of accel Modes
} ADXL375BCCZ_ACCEL_Mode;
//~ Type for accelerometer raw Sample
typedef struct ADXL375BCCZ_RawSample_str
{
int32_t sample_x; //~ X axis Sample
int32_t sample_y; //~ Y axis Sample
int32_t sample_z; //~ Z axis Sample
int32_t angle; //~ Calculated tilt
} Adxl375bcczRawSample;
24--Использовать повторяющиеся вызовы одной и той же конструкции кода вместо использования массива конфигурационных структур.
void PortInit(void) {
PORT_InitType tInitStruct = {0};
/* Port A19: MUX = ALT3, UART1_RX */
tInitStruct.u32PortPins = PORT_PIN_19;
tInitStruct.uPortPinMux.u32PortPinMode = PORTA_19_FCUART1_RX;
tInitStruct.bPullEn = true;
tInitStruct.ePullSel = PORT_PULL_UP;
PORT_InitPins(&g_tPortAHandle, &tInitStruct);
/* Port A18: MUX = ALT3, UART1_TX */
tInitStruct.u32PortPins = PORT_PIN_18;
tInitStruct.uPortPinMux.u32PortPinMode = PORTA_18_FCUART1_TX;
PORT_InitPins(&g_tPortAHandle, &tInitStruct);
/* LED2: PortC 31: MUX = GPIO output */
tInitStruct.u32PortPins = PORT_PIN_31;
tInitStruct.uPortPinMux.u32PortPinMode = PORT_GPIO_MODE;
tInitStruct.ePortGpioDir = PORT_GPIO_OUT;
tInitStruct.ePortGpioLevel = PORT_GPIO_LOW;
PORT_InitPins(&g_tPortDHandle, &tInitStruct);
/* LED0: PortA 26: MUX = GPIO output */
tInitStruct.u32PortPins = PORT_PIN_26;
PORT_InitPins(&g_tPortAHandle, &tInitStruct);
tInitStruct.u32PortPins = PORT_PIN_17;
tInitStruct.uPortPinMux.u32PortPinMode = PORTE_17_FCSMU_PIN1;
PORT_InitPins(&g_tPortEHandle, &tInitStruct);
tInitStruct.u32PortPins = PORT_PIN_16;
tInitStruct.uPortPinMux.u32PortPinMode = PORTA_16_FCSMU_PIN0;
PORT_InitPins(&g_tPortAHandle, &tInitStruct);
}5--Несостыковка по типам данных
6--Человек вызывает печать в UART из обработчика прерываний. Прерывание, которое генерирует другое прерывание. Здорово!
10--Транслит в названиях функций. Проблема транслита в том, что он у всех и каждого свой. time_metka.
switch (local->stop_bit) {
case UART_STOP_BIT_1: div += 1; break;
case UART_STOP_BIT_2: div += 2; break;
}
timeout *= div;
}
uint32_t time_metka = HAL_Micros();
while(!HAL_USART_RXNE_ReadFlag(local)) {
if (HAL_Micros() - time_metka > timeout) return false;
}
*buf = HAL_USART_ReadByte(local);
return true;
}
А тут fizical_address
12--Нет смысла в битовых полях, если тип совпадает с базовыми типами. Можно оставить uint8_t
13--Несовпадение типов в битовых полях.
14--Невнимательное заполнение битовых полей
15--Очевидные комментарии
16--Отдельная ветка if для нулевого значения, когда нулевое значение отлично может и обработать формула
18-- Журнальные комментарии. Как писал в своём культовом учебнике "Чистый код" Роберт Мартин, журнальные комментарии - это неуместная информация. Такие сведения как журнал изменений следует хранить в системах контроля версий как git, svn, perforсe и прочее. В исходниках эти метаданные только загромождают код.
//~***************************************************************************
//~ Sowftware component: GPIO
//~ file: gpio.c
//~ description This file contains functions used by the GPIO module.
//~ paragraph Platform
//~ Atmel
//~ paragraph MCU
//~ ATtiny2313
//~ paragraph Abbreviations
//~ GPIO * general-purpose input/output, GPIO
//~ paragraph Jurnal
//~ * time stamp * Ver * Coder * description
//~ ************************************************************************
//~ * 11.06.2012 * 1 * XUY * Add interrupt support
//~ * 55.06.2011 * 2 * ABC * Add missing pins
//~ * 17.04.2010 * 4 * LOH * Add config check.
//~ * * * * This flag is buggy.
//~ * 29.13.2009 * 6 * MUD * Add pull modes.
//~ * 14.02.2008 * 7 * QWE * Init vervion
//~******************************************************************************
/********************************************************************************
* Revision History:
*
* Version Date Initials CR# Descriptions
* --------- ---------- ------------ ---------- ---------------
* 0.1.0 31/12/2022 Flagchip071 N/A First version for FC7300
* 0.2.0 4/2/2022 Flagchip071 N/A 0.2.0 release
********************************************************************************/Много комментариев в коде это как самолет в чрезмерно большим ящиком для багажа.
17--Перечисления маленькими буквами. Как и всякие константы, экземпляры перечислений тоже лучше прописывать заглавными буквами. Чтобы не путать с переменными.
18- Не надо делать таблицу в перечислении. Если это линейная функция, то её следует вычислять по формуле.
19-- Беcсмыссленный if(0). Уж лучше препроцессором #ifdef делать. Правда?
20--Беcсмыссленный if(1). Если убрать if(1) то ничего не изменится.
21--Использование лесенки else if вместо оператора switch().
22-- Вставка include-ом *.c файликов! Это просто клиника...
23-- Длиннющие пути к заголовочным файлам. Вот перекинет кто файл simcom_modem_lib.h в другую папку и код перестанет собираться. Поэтому не надо так делать.
Вывод
Как видно, к нелепостям приводит необоснованная спешка написания, copy-paste-программирование и невнимательность. Не делайте так.
Присылайте в комментариях примеры "лучшего кода", который видели Вы.
Ссылки
№ | Название | URL |
2 | Атрибуты Хорошего С-кода | |
3 | КодоГенератор Линейных Отображений (как ускорить создание ASIC драйвера) |
