【show-me-bad-code】改善既有代码的设计，谈谈我的真实案例
我来提供一段代码代码背景
这段代码的原型来自于早几年在一家POS行业内头部企业供职时的真实案例代码，基于这套代码也是过了各种POS机的行业认证，
经过10多年的沉淀，基于该模板代码移植/改造而来的应用程序，罐装的POS机累计装机量应该在KW级别，可谓就是传说中妥妥的【祖传代码】。
早期代码的运行环境是嵌入式Linux系统，总内存高达128MB，还是非常富余的；后面才慢慢切为RTOS系统，内存也锐减到64MB，但即便这样，给到应用程序的内存也是非常充足的。
2.2 代码功能
这段代码的主要功能就是在POS机触发交易时的界面（带LCD显示）下，能够自动识别出当前要使用的各种交易方式，其中包括：
是否刷了磁条卡？（刷了磁条卡，走刷磁条卡的交易流程）
是否插入了IC卡？（插入IC卡，走银联的IC卡交易流程，行内称PBOC流程）
是否使用了IC卡做非接触交易（俗称：挥卡）？（挥卡交易，走银联的IC卡快速交易，行内称qPBOC流程）
是否扫描到了微信/支付宝这类的支付条码？（条码支付与标准银**交易是完全不一样的流程）
是否按下了【取消键】，用户主动退出了交易？
是否在规定的时间内（一般60S）没有任何的交易发生，导致超时？
除这些功能外，因为POS机交易是需要保存最近N笔交易记录的，所以这个处理交易的流程中，还得维护交易记录的保存。而代码中用于保存交易记录的结构体也是非常非常非常的庞大，大到一个结构体就占用接近2KB，这是非常恐怖的。
代码片段
为了仅说明情况，而不透露具体的技术细节，这里我仅提供伪代码，注意代码里的注释，是我为了辅助介绍而自己添加的，原来的代码中，注释比这少得可怜。
/**
* 保存交易记录的结构体定义
*/
typedef struct _transaction_log_t {
char time_stamp;
uint8_t trans_type;
uint8_t trans_no;
/* 各式各样的数据定义长达20-30个变量 */
uint8_t data1;
...
uint8_t datan;
} transaction_log_t;
/* 全局的交易log变量，被各个处理流程的文件引用 */
transaction_log g_log_info;
/**
* 支持的交易方式的掩码
*/
#define TRANSACTION_SWIPE_CARD 0x01
#define TRANSACTION_INSERT_CARD 0x02
#define TRANSACTION_TAP_CARD 0x04
#define TRANSACTION_CODE_PAY 0x08
/**
* 处理交易的流程总入口
*/
int all_transaction_process(int timeouts_ms, int transaction_flag)
{
int start_time, end_time;
end_time = 0;
start_time = get_local_time();
/* 循环判断需要支持的交易方式，直到触发了某种交易或者超时 */
while (1) {
if (transaction_flag & TRANSACTION_SWIPE_CARD) {
/* 判断刷卡：阻塞式的接口，带超时时间 */
ret = get_swipe_card(10);
if (ret == ok) {
/* 处理刷卡流程 */
...
return 0;
}
} else if (transaction_flag & TRANSACTION_SWIPE_CARD) {
/* 判断插卡：阻塞式接口，带超时时间 */
ret = get_insert_card(10);
if (ret == ok) {
/* 处理插卡流程 */
...
return 0;
}
} else if (transaction_flag & TRANSACTION_SWIPE_CARD) {
/* 判断挥卡：阻塞式接口，带超时时间 */
ret = get_tap_card(10);
if (ret == ok) {
/* 处理挥卡流程 */
...
return 0;
}
} else if (transaction_flag & TRANSACTION_SWIPE_CARD) {
/* 判断条码支付 */
ret = get_code(10);
if (ret == ok) {
/* 处理条码流程 */
...
return 0;
}
}
end_time = get_local_time();
if (start_time + timeous_ms == end_time) {
/* timeout */
return -1;
}
/* 界面显示倒计时 */
lcd_update_timetous();
/* 循环延时 */
delay_ms(10);
}
}
/**
* main函数总入口
*/
int main(int argc, const char *argv[])
{
/* 应用初始化 */
system_init();
/* 处理备份及系统异常断电引发的冲正交易 */
handle_bakup();
/* main loop */
while (1) {
/* 每10ms获取下当前有没有按键触发交易 */
key = get_key_ms(10);
if (key1 == key) {
/* 有触发交易需求 */
ret = all_transaction_process(60*1000, );
} else if (key2 == key) {
/* 其他菜单操作 */
...
} else {
/* 超时或未知按键输入 */
...
}
/* 刷新显示在LCD上的时间 */
update _time_dispaly();
}
return 0;
}
"烂"的理由
这里说上面的代码"烂"并不是说它工作得不行，相反，它的确工作得可以，从装机量和各种行业认证就可以看得出来，还是扛得住市场对它的考验。这里提出它"烂"的原因，主要考虑是从代码设计和代码维护的角度来看。
代码维护上：全局变量全盘引用，乱串于各个文件中，可读性非常差
代码设计上：数据结构没有规划设计，导致结构体定义太复杂，太过庞然大物，冗余空间浪费大
代码设计上：所有交易流程的判断处理，太过于死板，一个if-else判断到底，耦合严重，可扩展性非常差
代码设计上：LCD显示与核心业务流程跳转参杂在一起，没有解耦，往往改了业务功能的同时还要改UI实现
代码性能上：流程处理中，大量使用带超时时间的阻塞式函数，导致整个流程响应的时效性不是很理想
改善代码前的思考
大家都戏称祖传代码，请勿随意改动，但我觉得真正优秀的代码，是慢慢被迭代，被重构而来的，否则的代码就会被一步步地堆砌起来，直到有一天，没人能够再修改维护了，那一刻就如同一栋大厦轰然倒塌，这是非常糟糕的。
正是基于这些的考虑，当时我也是很大胆地向主管提出，我们应该从小面积开始做代码重构，逐渐改善一些有缺陷的代码设计。
比较遗憾的是，主管是相对保守的，没有接受后面大面积重构的实施，主要还是担心期间的风险，影响外面的程序升级。从商业的角度上，我是支持他的；但是，从代码的角度，我还是**我的观点；当然这并不影响我们的共事。
我要如何改善这些代码
先说下，主管答允我的小面积重构部分，我是怎么做的！
原文链接：https://club.rt-thread.org/ask/article/3113.html
页:
[1]