发新帖本帖赏金 0.05元(功能说明)我要提问
返回列表
打印
[嵌入式linux]

【show-me-bad-code】改善既有代码的设计,谈谈我的真实案例

[复制链接]
3623|0
手机看帖
扫描二维码
随时随地手机跟帖
跳转到指定楼层
楼主
RTThread小师弟|  楼主 | 2021-10-29 17:12 | 只看该作者 回帖奖励 |倒序浏览 |阅读模式
我来提供一段代码
代码背景

这段代码的原型来自于早几年在一家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[32];
    uint8_t trans_type;
    uint8_t trans_no;
   
    /* 各式各样的数据定义长达20-30个变量 */
    uint8_t data1[64];
    ...
    uint8_t datan[128];
} 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

使用特权

评论回复

打赏榜单

小胡子啊 打赏了 0.05 元 2021-10-29

相关帖子

发新帖 本帖赏金 0.05元(功能说明)我要提问
您需要登录后才可以回帖 登录 | 注册

本版积分规则

1

主题

1

帖子

0

粉丝